more aggressive version that allows PhiDefs

AndyAyersMS · Aug 12, 2024 · 72f2097 · 72f2097
1 parent e011f09
commit 72f2097
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 5 deletions.
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
@@ -6999,7 +6999,7 @@ class Compiler
     // Returns true iff the ValueNum "vn" represents a value that is loop-invariant in "loop".
     //   Constants and init values are always loop invariant.
     //   VNPhi's connect VN's to the SSA definition, so we can know if the SSA def occurs in the loop.
-    bool optVNIsLoopInvariant(ValueNum vn, FlowGraphNaturalLoop* loop, VNSet* recordedVNs);
+    bool optVNIsLoopInvariant(ValueNum vn, FlowGraphNaturalLoop* loop, VNSet* recordedVNs, bool ignorePhiDefs = false);
 
     // Records the set of "side effects" of all loops: fields (object instance and static)
     // written to, and SZ-array element type equivalence classes updated.

diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
@@ -5147,7 +5147,23 @@ void Compiler::optHoistCandidate(GenTree*              tree,
     Metrics.HoistedExpressions++;
 }
 
-bool Compiler::optVNIsLoopInvariant(ValueNum vn, FlowGraphNaturalLoop* loop, VNSet* loopVnInvariantCache)
+//------------------------------------------------------------------------------
+// optVNIsLoopInvariant: Check if the value(s) this VN refers to may vary
+//  across iterations of a loop
+//
+// Arguments:
+//   vn -- the vn of interest
+//   loop -- the loop
+//   loopVnInvariantCache -- cache of prior results for this loop
+//   ignorePhiDefs -- if true, only consider memory dependence
+//
+// Returns:
+//   true if this VN represents a loop invariant
+//
+bool Compiler::optVNIsLoopInvariant(ValueNum              vn,
+                                    FlowGraphNaturalLoop* loop,
+                                    VNSet*                loopVnInvariantCache,
+                                    bool                  ignorePhiDefs)
 {
     // If it is not a VN, is not loop-invariant.
     if (vn == ValueNumStore::NoVN)
@@ -5226,7 +5242,7 @@ bool Compiler::optVNIsLoopInvariant(ValueNum vn, FlowGraphNaturalLoop* loop, VNS
                 // TODO-CQ: We need to either make sure that *all* VN functions
                 // always take VN args, or else have a list of arg positions to exempt, as implicitly
                 // constant.
-                if (!optVNIsLoopInvariant(funcApp.m_args[i], loop, loopVnInvariantCache))
+                if (!optVNIsLoopInvariant(funcApp.m_args[i], loop, loopVnInvariantCache, ignorePhiDefs))
                 {
                     res = false;
                     break;
@@ -5238,7 +5254,7 @@ bool Compiler::optVNIsLoopInvariant(ValueNum vn, FlowGraphNaturalLoop* loop, VNS
     {
         // Is the definition within the loop?  If so, is not loop-invariant.
         LclSsaVarDsc* ssaDef = lvaTable[phiDef.LclNum].GetPerSsaData(phiDef.SsaDef);
-        res                  = !loop->ContainsBlock(ssaDef->GetBlock());
+        res                  = ignorePhiDefs || !loop->ContainsBlock(ssaDef->GetBlock());
     }
     else if (vnStore->GetMemoryPhiDef(vn, &memoryPhiDef))
     {

diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp
@@ -11165,7 +11165,8 @@ void Compiler::fgValueNumberPhiDef(GenTreeLclVar* newSsaDef, BasicBlock* blk, bo
         if (isUpdate && (phiArgVNP != phiArg->gtVNPair))
         {
             FlowGraphNaturalLoop* const blockLoop = m_loops->GetLoopByHeader(blk);
-            bool const canUseNewVN = optVNIsLoopInvariant(phiArgVNP.GetConservative(), blockLoop, &loopInvariantCache);
+            bool const canUseNewVN = optVNIsLoopInvariant(phiArgVNP.GetConservative(), blockLoop, &loopInvariantCache,
+                                                          /* ignorePhiDefs */ true);
 
             if (canUseNewVN)
             {