[1.10>master] [MERGE #5535 @rajatd] Don't add compensation code for array values in prepass. OS#17527968

Merge pull request #5535 from rajatd:avt-array

With aggressive transfer of values in loop prepass, an assignment of an array will result in an Array kind value for the dst (without value transfer in prepass, it would have been a Generic kind value).

Globopt, when trying to merge two Array kind values from two edges, will try to insert compensation code if, for example, the length sym on the two values is different. We are currently hitting an assert that we should only be inserting compensation code in the main pass of the loop. Before AVT, we wouldn't even have tried to insert compensation because the value types on the two edges would be different.

Given the timeframe and release being shutdown, I'm changing the code so it only inserts compensation code in the main pass (instead of inserting compensation in prepass). This matches the behaviour to what we had before AVT.

Thanks @meg-gupta for the reduction on this one.

Author: rajatd

lib/Backend/GlobOptBlockData.cpp

@@ -1086,7 +1086,7 @@ ValueInfo *GlobOptBlockData::MergeArrayValueInfo(
     // but in different syms, create a new sym and record that the array sym requires compensation. Compensation will be
     // inserted later to initialize this new sym from all predecessors of the merged block.
 
-    StackSym *newHeadSegmentSym;
+    StackSym *newHeadSegmentSym = nullptr;
     if(toDataValueInfo->HeadSegmentSym() && fromDataValueInfo->HeadSegmentSym())
     {
         if(toDataValueInfo->HeadSegmentSym() == fromDataValueInfo->HeadSegmentSym())
@@ -1095,27 +1095,26 @@ ValueInfo *GlobOptBlockData::MergeArrayValueInfo(
         }
         else
         {
-            Assert(!this->globOpt->IsLoopPrePass());
-            Assert(symsRequiringCompensation);
-            symsRequiringCompensation->Set(arraySym->m_id);
-            Assert(symsCreatedForMerge);
-            if(symsCreatedForMerge->Test(toDataValueInfo->HeadSegmentSym()->m_id))
+            if (!this->globOpt->IsLoopPrePass())
             {
-                newHeadSegmentSym = toDataValueInfo->HeadSegmentSym();
-            }
-            else
-            {
-                newHeadSegmentSym = StackSym::New(TyMachPtr, this->globOpt->func);
-                symsCreatedForMerge->Set(newHeadSegmentSym->m_id);
+                // Adding compensation code in the prepass won't help, as the symstores would again be different in the main pass.
+                Assert(symsRequiringCompensation);
+                symsRequiringCompensation->Set(arraySym->m_id);
+                Assert(symsCreatedForMerge);
+                if (symsCreatedForMerge->Test(toDataValueInfo->HeadSegmentSym()->m_id))
+                {
+                    newHeadSegmentSym = toDataValueInfo->HeadSegmentSym();
+                }
+                else
+                {
+                    newHeadSegmentSym = StackSym::New(TyMachPtr, this->globOpt->func);
+                    symsCreatedForMerge->Set(newHeadSegmentSym->m_id);
+                }
             }
         }
     }
-    else
-    {
-        newHeadSegmentSym = nullptr;
-    }
 
-    StackSym *newHeadSegmentLengthSym;
+    StackSym *newHeadSegmentLengthSym = nullptr;
     if(toDataValueInfo->HeadSegmentLengthSym() && fromDataValueInfo->HeadSegmentLengthSym())
     {
         if(toDataValueInfo->HeadSegmentLengthSym() == fromDataValueInfo->HeadSegmentLengthSym())
@@ -1124,27 +1123,25 @@ ValueInfo *GlobOptBlockData::MergeArrayValueInfo(
         }
         else
         {
-            Assert(!this->globOpt->IsLoopPrePass());
-            Assert(symsRequiringCompensation);
-            symsRequiringCompensation->Set(arraySym->m_id);
-            Assert(symsCreatedForMerge);
-            if(symsCreatedForMerge->Test(toDataValueInfo->HeadSegmentLengthSym()->m_id))
+            if (!this->globOpt->IsLoopPrePass())
             {
-                newHeadSegmentLengthSym = toDataValueInfo->HeadSegmentLengthSym();
-            }
-            else
-            {
-                newHeadSegmentLengthSym = StackSym::New(TyUint32, this->globOpt->func);
-                symsCreatedForMerge->Set(newHeadSegmentLengthSym->m_id);
+                Assert(symsRequiringCompensation);
+                symsRequiringCompensation->Set(arraySym->m_id);
+                Assert(symsCreatedForMerge);
+                if (symsCreatedForMerge->Test(toDataValueInfo->HeadSegmentLengthSym()->m_id))
+                {
+                    newHeadSegmentLengthSym = toDataValueInfo->HeadSegmentLengthSym();
+                }
+                else
+                {
+                    newHeadSegmentLengthSym = StackSym::New(TyUint32, this->globOpt->func);
+                    symsCreatedForMerge->Set(newHeadSegmentLengthSym->m_id);
+                }
             }
         }
     }
-    else
-    {
-        newHeadSegmentLengthSym = nullptr;
-    }
 
-    StackSym *newLengthSym;
+    StackSym *newLengthSym = nullptr;
     if(toDataValueInfo->LengthSym() && fromDataValueInfo->LengthSym())
     {
         if(toDataValueInfo->LengthSym() == fromDataValueInfo->LengthSym())
@@ -1153,25 +1150,23 @@ ValueInfo *GlobOptBlockData::MergeArrayValueInfo(
         }
         else
         {
-            Assert(!this->globOpt->IsLoopPrePass());
-            Assert(symsRequiringCompensation);
-            symsRequiringCompensation->Set(arraySym->m_id);
-            Assert(symsCreatedForMerge);
-            if(symsCreatedForMerge->Test(toDataValueInfo->LengthSym()->m_id))
-            {
-                newLengthSym = toDataValueInfo->LengthSym();
-            }
-            else
+            if (!this->globOpt->IsLoopPrePass())
             {
-                newLengthSym = StackSym::New(TyUint32, this->globOpt->func);
-                symsCreatedForMerge->Set(newLengthSym->m_id);
+                Assert(symsRequiringCompensation);
+                symsRequiringCompensation->Set(arraySym->m_id);
+                Assert(symsCreatedForMerge);
+                if (symsCreatedForMerge->Test(toDataValueInfo->LengthSym()->m_id))
+                {
+                    newLengthSym = toDataValueInfo->LengthSym();
+                }
+                else
+                {
+                    newLengthSym = StackSym::New(TyUint32, this->globOpt->func);
+                    symsCreatedForMerge->Set(newLengthSym->m_id);
+                }
             }
         }
     }
-    else
-    {
-        newLengthSym = nullptr;
-    }
 
     if(newHeadSegmentSym || newHeadSegmentLengthSym || newLengthSym)
     {

test/Optimizer/Miscellaneous_MaxInterpret.js

@@ -1071,3 +1071,23 @@ function test66() {
 }
 WScript.Echo("test66: " + test66());
 WScript.Echo("test66: " + test66());
+
+(function test67Runner(){
+    function test67(a,b,x){
+        var i = a[0];
+        var j = b[0];
+        var t = a;
+        for (var p = 0; p < x; p++) {
+            if (p > 2) {
+                t = b;
+                b = a;
+            }
+        }
+    }
+
+    var arr1 = [1,2];
+    var arr2 = [3,4];
+    var x = 1;
+    test67(arr1, arr2, x);
+    test67(arr1, arr2, x)
+})();