Forbid - byref cnst -> + (byref -cnst) transformation.

[sandreenko]

This came up during #43386 work,
if we declare a method that subs 2 addresses and returns native int:

 .method public static native int byrefsubbyref(class ctest& V_1, class ctest& V_2) aggressiveinlining
{
ldarg 0
ldarg 1
sub
ret
}

and pass to it a local var addr (will be represented as long on x64) and a const byref:

 [000040] ------------ * SUB long [000037] ------------ +--* ADDR long // not sure why this is not a byref...
[000038] -------N---- | \--* LCL_VAR ref V00 loc0 [000039] H----------- \--* CNS_INT(h) byref 0x1f6a1802cb8 static Fseq[S_1]

and if morph transforms SUB to ADD with - on [000039] we will get:

N004 ( 6, 14) [000040] ------------ \--* ADD long $300
N002 ( 3, 3) [000037] ------------ +--* ADDR long $2c0
N001 ( 3, 2) [000038] ----G--N---- | \--* LCL_VAR ref (AX) V00 loc0 $280
N003 ( 2, 10) [000039] H----------- \--* CNS_INT(h) byref -0x1f6a1802cb8 static $142

then this tree will trigger an assert Make sure a GC address doesn't end up in 'rv2' when we try to create an addr mode from it.

The PR fixes it by forbidding SUB(A, CNST) -> ADD(A, -CNST) transformation for byrefs.

oldJit.NewTest.BeforeTheFix.txt
oldJit.NewTest.AfterTheFix.txt

[sandreenko]

PTAL @AndyAyersMS @dotnet/jit-contrib , I added a new IL test that fails with an assert during compilation and proposed a fix for it. Please check that IL test looks correct to you and if you see more potential issues like that with other transformations.

[MichalStrehovsky]

Do you need conv.i4 before returning the value? Stack has native int, but method returns int32.

If you need the method to return 100, you could allocate a 101 element array and subtract the managed reference between the first element and last element.

The difference between a stack location and a static field that this is returning right now is a very random number.

[sandreenko]

The difference between a stack location and a static field that this is returning right now is a very random number.

You are right, I will fix the test to always return 100 instead of a random number.

I am not an expert in ECMA so I am very glad that you are looking at the test, thank you.
My understanding is that ECMA allows implicit coercion of native int to int so I do not need a cast there.
However, I could read it wrong and can add a cast just for safety.

from ECMA, page 38:
I.8.7.3 General assignment compatibility:

[CarolEidt]

I believe that @sandreenko is correct that this implicit conversion is allowed.