From 837a1c3d56a7b2d1706a5c5dc2b6df0836f8f6d5 Mon Sep 17 00:00:00 2001
From: Alex Covington <68252706+alexcovington@users.noreply.github.com>
Date: Fri, 19 Feb 2021 09:55:18 -0800
Subject: [PATCH] No const arg to temp (#48308)

* Starting to add IsInvariant helper

* Finished IsInvariant helper. Starting testing

* Removed IsInvariant from GenTree to avoid asserts

* Formatting

* Fixed missing not operator

* Changed AsOp() call to 'this' to avoid failing assertions

* Formatting

* Update src/coreclr/jit/morph.cpp

Co-authored-by: Sergey Andreenko <seandree@microsoft.com>

* Update src/coreclr/jit/compiler.h

Co-authored-by: Sergey Andreenko <seandree@microsoft.com>

* Apply suggestions from code review

Co-authored-by: Sergey Andreenko <seandree@microsoft.com>

* Change arguments to const to avoid cast

* Formatting

* delete `IsInvariant` from Compiler.

Co-authored-by: Sergey Andreenko <seandree@microsoft.com>
---
 src/coreclr/jit/compiler.h   | 3 +--
 src/coreclr/jit/gentree.cpp  | 8 +++++++-
 src/coreclr/jit/gentree.h    | 8 +++++---
 src/coreclr/jit/importer.cpp | 4 ++--
 src/coreclr/jit/morph.cpp    | 4 +---
 5 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index b69c0c1..51cf6d64 100644
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -4368,7 +4368,7 @@ private:
     static LONG jitNestingLevel;
 #endif // DEBUG
 
-    static BOOL impIsAddressInLocal(GenTree* tree, GenTree** lclVarTreeOut);
+    static BOOL impIsAddressInLocal(const GenTree* tree, GenTree** lclVarTreeOut);
 
     void impMakeDiscretionaryInlineObservations(InlineInfo* pInlineInfo, InlineResult* inlineResult);
 
@@ -10396,7 +10396,6 @@ public:
     GenTree* fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntryPtr);
 
     bool killGCRefs(GenTree* tree);
-
 }; // end of class Compiler
 
 //---------------------------------------------------------------------------------------------------------------------
diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index c359bef..794a1ef 100644
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -2845,7 +2845,7 @@ bool Compiler::gtCanSwapOrder(GenTree* firstNode, GenTree* secondNode)
             if (firstNode->gtFlags & strictEffects & GTF_PERSISTENT_SIDE_EFFECTS)
             {
                 // We have to be conservative - can swap iff op2 is constant.
-                if (!secondNode->OperIsConst())
+                if (!secondNode->IsInvariant())
                 {
                     canSwap = false;
                 }
@@ -19598,3 +19598,9 @@ bool GenTreeLclFld::IsOffsetMisaligned() const
     return false;
 }
 #endif // TARGET_ARM
+
+bool GenTree::IsInvariant() const
+{
+    GenTree* lclVarTree = nullptr;
+    return OperIsConst() || Compiler::impIsAddressInLocal(this, &lclVarTree);
+}
diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
index d29db8b..b475504 100644
--- a/src/coreclr/jit/gentree.h
+++ b/src/coreclr/jit/gentree.h
@@ -2189,10 +2189,12 @@ private:
 public:
     bool Precedes(GenTree* other);
 
+    bool IsInvariant() const;
+
     bool IsReuseRegVal() const
     {
         // This can be extended to non-constant nodes, but not to local or indir nodes.
-        if (OperIsConst() && ((gtFlags & GTF_REUSE_REG_VAL) != 0))
+        if (IsInvariant() && ((gtFlags & GTF_REUSE_REG_VAL) != 0))
         {
             return true;
         }
@@ -2200,12 +2202,12 @@ public:
     }
     void SetReuseRegVal()
     {
-        assert(OperIsConst());
+        assert(IsInvariant());
         gtFlags |= GTF_REUSE_REG_VAL;
     }
     void ResetReuseRegVal()
     {
-        assert(OperIsConst());
+        assert(IsInvariant());
         gtFlags &= ~GTF_REUSE_REG_VAL;
     }
 
diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index 63c4c37..e8f12dc 100644
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -18921,7 +18921,7 @@ bool Compiler::impIsValueType(typeInfo* pTypeInfo)
 
  */
 
-BOOL Compiler::impIsAddressInLocal(GenTree* tree, GenTree** lclVarTreeOut)
+BOOL Compiler::impIsAddressInLocal(const GenTree* tree, GenTree** lclVarTreeOut)
 {
     if (tree->gtOper != GT_ADDR)
     {
@@ -19450,7 +19450,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo*   pInlineInfo,
         INDEBUG(curArgVal->AsLclVar()->gtLclILoffs = argNum;)
     }
 
-    if ((curArgVal->OperKind() & GTK_CONST) || isAddressInLocal)
+    if (curArgVal->IsInvariant())
     {
         inlCurArgInfo->argIsInvariant = true;
         if (inlCurArgInfo->argIsThis && (curArgVal->gtOper == GT_CNS_INT) && (curArgVal->AsIntCon()->gtIconVal == 0))
diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index 94f9863..0c07628 100644
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -1340,9 +1340,7 @@ void fgArgInfo::ArgsComplete()
                 fgArgTabEntry* prevArgTabEntry = argTable[prevInx];
                 assert(prevArgTabEntry->argNum < curArgTabEntry->argNum);
 
-                // TODO-CQ: We should also allow LCL_VAR_ADDR and LCL_FLD_ADDR here, they're
-                // side effect free leaf nodes that like constant can be evaluated at any point.
-                if (prevArgTabEntry->GetNode()->gtOper != GT_CNS_INT)
+                if (!prevArgTabEntry->GetNode()->IsInvariant())
                 {
                     prevArgTabEntry->needTmp = true;
                     needsTemps               = true;
-- 
2.7.4