[WIP/Test pr] Fix GTF_ASG flag on call nodes.

[sandreenko]

We were artificially added GTF_ASG flag into treeFlags in fgDebugCheckFlags to avoid asserts about missing flags.
However, it created an inconsistency with gtUpdateSideEffects. It correctly propagated ASG flag so we had a situation when calling gtUpdateSideEffects(tree, stmt) for an unchanged tree could result in changed flags:
before calling gtUpdateSideEffects(000055, stmt):

N019 ( 51, 40) [000068] --CXG-------              *  JTRUE     void
N018 ( 49, 38) [000067] J-CXG--N----              \--*  LT        int    $148
N016 ( 47, 36) [000065] --CXG------- HERE            +--*  CALLV stub int    System.Collections.IComparer.Compare $383
N010 ( 18, 11) [000131] -ACXG---R-L- arg2 SETUP      |  +--*  ASG       ref    $346
N009 (  1,  1) [000130] D------N----                 |  |  +--*  LCL_VAR   ref    V12 tmp5         d:2 $346
N008 ( 18, 11) [000059] --CXG-------                 |  |  \--*  CALL r2r_ind ref    System.Array.GetValue $346
N006 (  3,  2) [000049] *--XG------- this in rcx     |  |     +--*  IND       ref    <l:$286, c:$285>
N005 (  1,  1) [000048] ------------                 |  |     |  \--*  LCL_VAR   byref  V00 this         u:1 Zero Fseq[keys] $80
N007 (  1,  1) [000055] ------------ arg1 in rdx     |  |     \--*  LCL_VAR   int    V05 loc2         u:5 (last use) $147
N012 (  1,  1) [000132] ------------ arg2 in rdx     |  +--*  LCL_VAR   ref    V12 tmp5         u:2 (last use) $346
N013 (  1,  1) [000064] ------------ arg3 in r8      |  +--*  LCL_VAR   ref    V04 loc1         u:2 $205
N014 (  1,  1) [000062] ------------ this in rcx     |  +--*  LCL_VAR   ref    V09 tmp2         u:2 (last use) <l:$241, c:$341>
N015 (  3, 10) [000127] ------------ arg1 in r11     |  \--*  CNS_INT(h) long   0xd1ffab1e ftn REG r11 $480
N017 (  1,  1) [000066] ------------                 \--*  CNS_INT   int    0 $40

after:

N019 ( 51, 40) [000068] -ACXG-------              *  JTRUE     void
N018 ( 49, 38) [000067] JACXG--N----              \--*  LT        int    $148
N016 ( 47, 36) [000065] -ACXG------- HERE                        +--*  CALLV stub int    System.Collections.IComparer.Compare $383
N010 ( 18, 11) [000131] -ACXG---R-L- arg2 SETUP      |  +--*  ASG       ref    $346
N009 (  1,  1) [000130] D------N----                 |  |  +--*  LCL_VAR   ref    V12 tmp5         d:2 $346
N008 ( 18, 11) [000059] --CXG-------                 |  |  \--*  CALL r2r_ind ref    System.Array.GetValue $346
N006 (  3,  2) [000049] *--XG------- this in rcx     |  |     +--*  IND       ref    <l:$286, c:$285>
N005 (  1,  1) [000048] ------------                 |  |     |  \--*  LCL_VAR   byref  V00 this         u:1 Zero Fseq[keys] $80
N007 (  1,  1) [000055] ------------ arg1 in rdx     |  |     \--*  LCL_VAR   int    V05 loc2         u:5 (last use) $147
N012 (  1,  1) [000132] ------------ arg2 in rdx     |  +--*  LCL_VAR   ref    V12 tmp5         u:2 (last use) $346
N013 (  1,  1) [000064] ------------ arg3 in r8      |  +--*  LCL_VAR   ref    V04 loc1         u:2 $205
N014 (  1,  1) [000062] ------------ this in rcx     |  +--*  LCL_VAR   ref    V09 tmp2         u:2 (last use) <l:$241, c:$341>
N015 (  3, 10) [000127] ------------ arg1 in r11     |  \--*  CNS_INT(h) long   0xd1ffab1e ftn REG r11 $480
N017 (  1,  1) [000066] ------------                 \--*  CNS_INT   int    0 $40

see that flags on [000065] have changed, but they should not.

Of course, there are many asm diffs from that change that should not exist.

[mikedn]

Fallout from my call arg list changes? 😟

[sandreenko]

Fallout from my call arg list changes? 😟

I don't think so. The strange code in fgDebugCheckFlags was existing before your changes.
unsigned treeFlags = tree->gtFlags & GTF_ALL_EFFECT; should be declared as const, so we don't add flags on the tree if they aren't really presented there.
And I don't see where setupArg flags were propagated in the old version before #24294.

[mikedn]

Right, it looks as if the fgDebugCheckFlags code was trying to compensate for the missing flag. And in general, side effect propagation doesn't seem to work properly for calls. For "simple" ops, the children are morphed and then side effects are propagated. For calls, fgMorphArgs collects the side effects while doing all kinds of stuff, copies side effects to the call node with something like:

call->gtFlags |= flagsSummary & GTF_ALL_EFFECT;

and then does more stuff, such as calling EvalArgsToTemps. Oh well... typical JIT fuzzy logic 😄.

[maryamariyan]

Thank you for your contribution. As announced in #27549 the dotnet/runtime repository will be used going forward for changes to this code base. Closing this PR as no more changes will be accepted into master for this repository. If you’d like to continue working on this change please move it to dotnet/runtime.