Clear GTF_ASG from non-stores in rationalize

[JosephTremoulet]

Since rationalization hoists comma antecedents out into separate
statements, any side-effects that the antecedent had might no longer apply
to the comma's ancestor nodes. Moreover, in LIR, side-effect flags need
only describe the immediate node; summary flags are computed when Ranges
are constructed. So, update rationalize to clear the GTF_ASG flag from
any node that is not a store. Failure to do so gives an
overly-conservative view of side-effects, which can inhibit dead store
removal.

Fixes #9633.

[pgavlin]

Is there any measurable throughput impact from this change?

[JosephTremoulet]

/cc @dotnet/jit-contrib

In addition to removing the three redundant loads noted in #9633 from ReadOnlySpan<T>..ctor, this removes other dead expressions here and there. Jit-diff reports (over full release tree):

Found files with textual diffs.

Summary:
(Note: Lower is better)

Total bytes of diff: -4367 (0.00 % of base)
    diff is an improvement.

Total byte diff includes 0 bytes from reconciling methods
        Base had    0 unique methods,        0 unique bytes
        Diff had    0 unique methods,        0 unique bytes

Top file improvements by size (bytes):
       -1162 : System.Private.CoreLib.dasm (-0.03 % of base)
        -649 : Directed\cmov\Bool_Or_Op_cs_do\Bool_Or_Op_cs_do.dasm (-0.32 % of base)
        -597 : Directed\cmov\Bool_Or_Op_cs_ro\Bool_Or_Op_cs_ro.dasm (-0.29 % of base)
        -293 : Directed\cmov\Bool_Xor_Op_cs_do\Bool_Xor_Op_cs_do.dasm (-0.13 % of base)
        -293 : Directed\cmov\Bool_Xor_Op_cs_ro\Bool_Xor_Op_cs_ro.dasm (-0.13 % of base)

34 total files with size differences (34 improved, 0 regressed).

Top method improvements by size (bytes):
        -189 : System.Private.CoreLib.dasm - Enumerator:System.Collections.IEnumerator.get_Current():ref:this (117 methods)
        -134 : System.Private.CoreLib.dasm - Dictionary`2:System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey,TValue>>.Contains(struct):bool:this (29 methods)
        -125 : System.Private.CoreLib.dasm - Dictionary`2:System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey,TValue>>.Remove(struct):bool:this (29 methods)
        -100 : System.Private.CoreLib.dasm - Enumerator:System.Collections.IDictionaryEnumerator.get_Entry():struct:this (29 methods)
         -65 : Directed\cmov\Bool_Or_Op_cs_do\Bool_Or_Op_cs_do.dasm - testout1:Sub_Funclet_301():int

352 total methods with size differences (352 improved, 0 regressed).

Here's an example diff, from Dictionary'2:System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey,TValue>>.Remove(struct):bool:this:

Before:

G_M8781_IG02:
       mov      rdx, qword ptr [rdi]
       mov      rdx, qword ptr [rdx+48]
       mov      rdx, qword ptr [rdx]
       mov      rdx, qword ptr [rdx+72]
       mov      rdx, gword ptr [rsi]
       mov      rcx, rdi
       call     Dictionary`2:FindEntry(ref):int:this

After:

G_M8781_IG02:
       mov      rdx, qword ptr [rdi]
       mov      rdx, gword ptr [rsi]
       mov      rcx, rdi
       call     Dictionary`2:FindEntry(ref):int:this

[JosephTremoulet]

@dotnet-bot test Windows_NT x64 throughput

[pgavlin]

@JosephTremoulet while you're making these changes, would it be too much to ask to also clear the GTF_CALL flag on nodes that are not calls? That may result in a few improvements as well.

[JosephTremoulet]

would it be too much to ask to also clear the GTF_CALL flag on nodes that are not calls

I'm happy to do that but would rather do the separate flags as separate changes since they could run into separate issues.

[pgavlin]

Seems reasonable to me.

[pgavlin]

LGTM

[briansull]

LGTM as well

[JosephTremoulet]

I believe the TP results are well within measurement noise, but will have @dotnet-bot test Windows_NT x64 throughput again just to cross-check, and test Tizen armel Cross Release Build since the timeout is presumably unrelated.

[JosephTremoulet]

@dotnet-bot test this please