Some GT_BLK/GT_OBJ related cleanup

[mikedn]

Initially this PR was attempting to reduce the usage of GT_BLK but since I can't finish it until repo consolidation I reduced it to some trivial cleanup of weird stuff I found while investigating differences between GT_OBJ and GT_BLK handling.

I also add a dump improvement - include class layout names in the dump. Something like:

STMT00000 (IL 0x000...0x007)
               [000006] -A-XG+------              *  ASG       struct (copy)
               [000005] ---XG+------              +--*  BLK       struct<SomeStruct, 24>
               [000009] -----+------              |  \--*  ADD       byref 
               [000000] -----+------              |     +--*  LCL_VAR   ref    V00 arg0         
               [000008] -----+------              |     \--*  CNS_INT   long   56 field offset Fseq[n]
               [000002] ---XG+------              \--*  IND       struct
               [000011] -----+------                 \--*  ADD       byref 
               [000001] -----+------                    +--*  LCL_VAR   ref    V01 arg1         
               [000010] -----+------                    \--*  CNS_INT   long   56 field offset Fseq[n]

While it makes dumps more verbose I find it quite confusing to see only "struct" and the size in dumps, without any obvious information about the struct type itself.

[maryamariyan]

Thank you for your contribution. As announced in dotnet/coreclr#27549 this repository will be moving to dotnet/runtime on November 13. If you would like to continue working on this PR after this date, the easiest way to move the change to dotnet/runtime is:

1. In your coreclr repository clone, create patch by running git format-patch origin
2. In your runtime repository clone, apply the patch by running git apply --directory src/coreclr <path to the patch created in step 1>

[mikedn]

@CarolEidt Any chance to get this in before repo consolidation? I'll get back to reducing GT_BLK usage when the new repo is open, for now this is just preparatory cleanup.

[mikedn]

Taking the address of something that isn't addressable has to be one of the most peculiar things in the JIT IR. It will be fun to figure out to get rid of GT_ADDR in this case, probably there aren't many options:

1. Attach class layout to SIMD/HWINTRINSIC nodes. Probably overkill considering the rather limited need for this, call args. Though it can be regarded as a "base type" replacement which isn't unreasonable.
2. Add some kind of node that has the same role as OBJ(ADDR(x)) without requiring addressing. Might work but I'm rather cautious about such "decorative" nodes.
3. Add class layout info to GenTreeCall::Use. IMO the best option.

[CarolEidt]

I agree that 3 is probably the best option.

[CarolEidt]

@mikedn have you measured the impact on JIT memory allocation? You indicate that you're planning to reduce BLK usage later, but I'm not crazy about increasing it even as a temporary change.

[mikedn]

have you measured the impact on JIT memory allocation?

Hmm, there's nothing in this PR that could increase memory usage. Actually there's a tiny decrease due to the elimination of some gtNewObjNode calls - GS was creating ADDR(LCL_VAR) trees and then gtNewObjNode was immediately discarding the ADDR nodes.

Mem usage diff:

--- D:\Projects\jit\mem-base.txt	2019-11-10 21:56:20.000000000 +-0200
+++ D:\Projects\jit\mem-diff.txt	2019-11-12 20:23:13.000000000 +-0200
@@ -11,16 +11,16 @@
 All allocations:
 For     31351 methods:
-  count:           26155164 (avg     834 per method)
-  alloc size :   2348905516 (avg   74922 per method)
+  count:           26155136 (avg     834 per method)
+  alloc size :   2348903276 (avg   74922 per method)
   max alloc  :        76048
 
   allocateMemory   :   3592421376 (avg  114587 per method)
-  nraUsed    :   2456020232 (avg   78339 per method)
+  nraUsed    :   2456017992 (avg   78339 per method)
 
 Alloc'd bytes by kind:
                   kind |       size |     pct
   ---------------------+------------+--------
          AssertionProp |  203592340 |   8.67%
-               ASTNode |  470228608 |  20.02%
+               ASTNode |  470226368 |  20.02%
               InstDesc |   60546312 |   2.58%
               ImpStack |   12556440 |   0.53%

[CarolEidt]

LGTM - thanks for the cleanup and the quick response to my question!

[CarolEidt]

Huh. I was thinking that this case would result in situations where we'd create an unnecessary OBJ(ADDR), however, I now see that this could never actually be hit!