Fix issue where resolution move would overwrite the source of operand of JCMP

[kunalspathak]

We were overwriting a register with resolution move which was the source of the operand used for JCMP. Special case it to also check if that is the case and filter out such registers from getting resolution done.

Fixes following jitstressregs failures on Windows/Linux arm64:

https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-47307-head-929ed24b5ccf4bf2b0/System.Linq.Parallel.Tests/console.89614fbb.log?sv=2019-07-07&se=2021-03-15T02%3A39%3A50Z&sr=c&sp=rl&sig=mHbQ6BCU%2BLYjzTmsd9DRDTMHtSP5LsCDsZpwjhqvgn4%3D

https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-47307-head-929ed24b5ccf4bf2b0/System.Threading.Channels.Tests/console.64cd2de3.log?sv=2019-07-07&se=2021-03-15T02%3A39%3A50Z&sr=c&sp=rl&sig=mHbQ6BCU%2BLYjzTmsd9DRDTMHtSP5LsCDsZpwjhqvgn4%3D

[kunalspathak]

@dotnet/jit-contrib

[kunalspathak]

@AndyAyersMS or @BruceForstall - can you please review this?

[AndyAyersMS]

Are locals and copies the only special cases that might happen here?

[AndyAyersMS]

Also switch_regs is not as descriptive as one might like, given that we now set it for things that are not switches.

[kunalspathak]

That's the only ones I could think of. COPY are special because we defer the code generation until the consuming node like here:

runtime/src/coreclr/jit/codegenarmarch.cpp

Lines 379 to 381 in cb606ad

	caseGT_COPY:
	// This is handled at the time we call genConsumeReg() on the GT_COPY
	break;

So in the test that I was investigating, the GT_COPY node's generation get deferred and happens after the generation of resolution move, at the time when the JCMP is generated.

Generating: N787 ( 34, 17) [000046] --CXGO------ t46 = * CALLV vt-ind int MiniBench.MyChannelWriter`1[Int32][System.Int32].TryWrite REG x0 $218
GC regs: 100001 {x0 x20} => 100000 {x20}
Call: GCvars=0000000000000008 {V00}, gcrefRegs=100000 {x20}, byrefRegs=80000 {x19}
[15] Rec call GC vars = 0000000000000008
IN000f: blr x21
/--* t46 int Generating: [000930] --CXGO------ t930 = * COPY int REG x21 (currently: ***not unused***, should be unused)
Generating: N789 ( 1, 2) [000047] -c---------- t47 = CNS_INT int 0 REG NA $40
; Generating: N001 ( 1, 1) [000937] ------------ t937 = LCL_VAR byref V00 this x19 REG x19
/--* t937 byref Generating: N002 ( 2, 2) [000938] ------------ t938 = * COPY byref REG x0 (currently: ***unused***, should be notunused)
IN0010: mov x0, x19
V00 in reg x19 is becoming dead [000937]
Live regs: 180000 {x19 x20} => 100000 {x20}
Byref regs: 80000 {x19} => 0000 {}
V00 in reg x0 is becoming live [000938]
Live regs: 100000 {x20} => 100001 {x0 x20}
Byref regs: 0000 {} => 0001 {x0}
/--* t930 int +--* t47 int Generating: N791 ( 36, 20) [000048] CNE---XGO-N---- * JCMP void REG NA
IN0011: mov w21, w0
IN0012: cbnz (LARGEJMP)L_M2478_BB37

Regarding switchRegs, I will rename it to consumedRegs.

[AndyAyersMS]

Not obvious to me why the copy needs special casing here -- could we not insert the resolution move between the copy and jump?

[kunalspathak]

Inserting the resolution move between the copy and jump should be fine for some cases, for example:

mov w21, w0mov x0, x19 ; <-- resolution movecbnz w21, G_M2478_IG37

but when we generate code for JCMP, it is tied with the copy and so the resolution move happens before the copy/jmp, like this:

mov x0, x20 ldr x21,[x20] ldr x21,[x21,#72] ldr x21,[x21,#40] blr x21mov x0, x19 ; move resolution overwrites `x0` before it gets copied.mov w21, w0 cbnz w21, G_M2478_IG37G_M2478_IG05: mov x19, x0 ....

[AndyAyersMS]

Maybe add a comment that the GT_COPY gets special handling in codegen and is conceptually merged with the operation that consumes its result, so both its input and output regs must be excluded from the set available for resolution.

And only switches and jcmp have input regs (and so can be fed by copies), so those are the only block-ending branches that need special handling.

Which makes me wonder if we might need similar logic for switches. It looks like we don't check if the input regs for switches come from GT_COPY. Switches are rare enough that we might not have good stress coverage.

[kunalspathak]

Maybe add a comment

Done.

wonder if we might need similar logic for switches

Possibly, but I am not confident to make that change before seeing a scenario, so lets postpone it for now.