Jit: fix issues with optMethodFlags

Closes #6368.

optMethodFlags is a member field of the Compiler object. It is used
during importation to flag IR that might benefit from earlyProp, and
used in earlyProp to skip execution if nothing is flagged. However,
there were a number of issues with the implementation.

First, the value was never initialized. So in CHK/DBG builds, it would
get set to 0xFFFFFFFF by the default allocator fill. In RELEASE builds
the value would be randomly set. This randomness could easily lead to
nondeterministic codegen in RELEASE. More on this subsequently.

Second, the value was not propagated during inlining. So if interesting
constructs only entered the root method via inlines, they potentially
might not trigger earlyProp.

Third, not all interesting constructs were flagged.

The JIT ORs flag values into optMethodFlags as it imports constructs
that should trigger earlyProp. It also re-uses the same compiler instance
in most cases. So, relatively quickly, even in release, all the relevant
flags end up set and earlyProp will always be run. Hence the window
for observing the nondeterministic is was limited to the first few
methods jitted by each compiler instance.

So to sum up: in DBG/CHK, early prop always runs, regardless of need.
In RELEASE, it runs unpredictably for the first few methods, then always
runs, regardless of need.

To see the nondeterminism in RELEASE, early methods would have to be free
of interesting constructs but pull them in via inlining (or contain
interesting constructs not currently flagged as such during importation).
This set of circumstances is evidently rare enough that the nondeterminism
has not been noted. Also, it is quite unlikely to lead to bad codegen,
just missed opportunities.

We might have caught this if we had reliable RELEASE/CHK diffing ready;
see for instance #5063.

The fix is to initialize optMethodFlags to zero in compInit, and merge in
the inlinee's copy during fgInsertInlineeBlocks. Logging tracks when the
inlinee's copy causes a flag update.

This was tricky to test for diffs because in CHK all that should happen is
that we run earlyProp less often. So any diff in CHK is a missed case of
flagging during importation. I found once such case via SPMI where the array
index is a constant, and added constant indices to the flagging set.

Because I also set the block flag, this both caused earlyProp to run
and also look at that block; the latter has triggered a fair number of
diffs, almost all of them beneficial (0.42% improvement in SPMI), winners
outnumber losers by about 50:1.

A typical example of a diff is that the null check below is removed.

```asm
       mov      rcx, qword ptr [(reloc)]
       mov      edx, 4
       call     CORINFO_HELP_NEWARR_1_VC
**     mov      edx, dword ptr [rax+8]
       mov      byte  ptr [rax+16], 255
       mov      byte  ptr [rax+17], 254
```

With this change in place, earlyProp how runs selectively based on need in
all configurations. We're still running earlyProp in all the cases where it
can make a difference, and finding more improvements when it does run, but
we're not running it in cases where it can't.

Author: AndyAyersMS

src/jit/compiler.cpp

@@ -1801,6 +1801,9 @@ void                Compiler::compInit(ArenaAllocator * pAlloc, InlineInfo * inl
     //Used by fgFindJumpTargets for inlining heuristics.
     opts.instrCount  = 0;
 
+    // Used to track when we should consider running EarlyProp
+    optMethodFlags = 0;
+
     for (unsigned i = 0; i < MAX_LOOP_NUM; i++)
     {
         AllVarSetOps::AssignNoCopy(this, optLoopTable[i].lpAsgVars, AllVarSetOps::UninitVal());

src/jit/compiler.h

@@ -5540,7 +5540,7 @@ protected :
     };
 
 #define OMF_HAS_NEWARRAY    0x00000001  // Method contains 'new' of an array
-#define OMF_HAS_NEWOBJ      0x00800002  // Method contains 'new' of an object type. 
+#define OMF_HAS_NEWOBJ      0x00000002  // Method contains 'new' of an object type.
 #define OMF_HAS_ARRAYREF    0x00000004  // Method contains array element loads or stores.
 #define OMF_HAS_VTABLEREF   0x00000008  // Method contains method table reference.
 
@@ -5557,8 +5557,6 @@ protected :
         OPK_OBJ_GETTYPE
     };
 
-    bool impHasArrayRef;
-
     bool       gtIsVtableRef(GenTreePtr tree);
     GenTreePtr getArrayLengthFromAllocation(GenTreePtr tree);
     GenTreePtr getObjectHandleNodeFromAllocation(GenTreePtr tree);
@@ -5569,7 +5567,6 @@ protected :
     bool       optDoEarlyPropForFunc();
     void       optEarlyProp();
 
-
 #if ASSERTION_PROP
     /**************************************************************************
      *               Value/Assertion propagation

src/jit/flowgraph.cpp

@@ -22232,6 +22232,22 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
     compNeedsGSSecurityCookie |= InlineeCompiler->compNeedsGSSecurityCookie;
     compGSReorderStackLayout  |= InlineeCompiler->compGSReorderStackLayout;
 
+    // Update optMethodFlags
+
+#ifdef DEBUG
+    unsigned optMethodFlagsBefore = optMethodFlags;
+#endif
+
+    optMethodFlags |= InlineeCompiler->optMethodFlags;
+
+#ifdef DEBUG
+    if (optMethodFlags != optMethodFlagsBefore)
+    {
+        JITDUMP("INLINER: Updating optMethodFlags --  root:%0x callee:%0x new:%0x\n",
+                optMethodFlagsBefore, InlineeCompiler->optMethodFlags, optMethodFlags);
+    }
+#endif
+
     // If there is non-NULL return, replace the GT_CALL with its return value expression,
     // so later it will be picked up by the GT_RET_EXPR node.
     if ((pInlineInfo->inlineCandidateInfo->fncRetType != TYP_VOID) || (iciCall->gtCall.gtReturnType == TYP_STRUCT))

src/jit/importer.cpp

@@ -10053,10 +10053,11 @@ void              Compiler::impImportBlockCode(BasicBlock * block)
 
             /* Mark the block as containing an index expression */
 
-            if  (op1->gtOper == GT_LCL_VAR)
+            if (op1->gtOper == GT_LCL_VAR)
             {
-                if  (op2->gtOper == GT_LCL_VAR ||
-                     op2->gtOper == GT_ADD)
+                if (op2->gtOper == GT_LCL_VAR ||
+                    op2->gtOper == GT_CNS_INT ||
+                    op2->gtOper == GT_ADD)
                 {
                     block->bbFlags |= BBF_HAS_INDX;
                     optMethodFlags |= OMF_HAS_ARRAYREF;
@@ -10281,10 +10282,11 @@ void              Compiler::impImportBlockCode(BasicBlock * block)
 
             // Mark the block as containing an index expression
 
-            if  (op3->gtOper == GT_LCL_VAR)
+            if (op3->gtOper == GT_LCL_VAR)
             {
-                if  (op1->gtOper == GT_LCL_VAR ||
-                     op1->gtOper == GT_ADD)
+                if (op1->gtOper == GT_LCL_VAR ||
+                    op1->gtOper == GT_CNS_INT ||
+                    op1->gtOper == GT_ADD)
                 {
                     block->bbFlags |= BBF_HAS_INDX;
                     optMethodFlags |= OMF_HAS_ARRAYREF;