Преглед изворни кода

Fix IV analysis when a loop modifies an induction variable of another sibling loop sharing the same loop parent

When there are two independent loops that share the same loop parent, and the second inner loop modifies the induction variable of the first inner loop, the induction variable's changes should be marked as indeterminate.
Failing which, incorrect induction variable info can flow into the parent loop, which can lead to incorrect optimizations and deadcode.

Example:
  var f = 137044716;
  for (var _strvar26 in VarArr0) {
    for (; ... ; f++) {
    }
    for (var ... in ...) {
      f = (e > _strvar0 & 255);
    }
  }

The induction variable f at the top of the outer loop ends up with incorrect bounds of (137044716, INT_MAX) incorrectly.
The induction variable analysis says that this loop has a induction variable with positive changed bounds.
Instead of marking it as indeterminate due to the & operation in the inner loop, because of this the min is taken as the max of the two merging lower bounds.
From this bad int range, blocks get optimized away and bad things happen.

The f++ operation in the first inner loop adds it as an induction variable and sets the bounds correctly.
The   f = (e > _strvar0 & 255); operation does not do any modification on the induction variable analysis.
At the end of this block this variable does not get detected as changing bounds because it was not in the block's induction variable list in the first place.
This change fixes this.

Fixes OS#12152925.
Meghana Gupta пре 8 година
родитељ
комит
a858c72271
3 измењених фајлова са 76 додато и 0 уклоњено
  1. 30 0
      lib/Backend/GlobOptIntBounds.cpp
  2. 41 0
      test/Optimizer/invalidIVRangeBug.js
  3. 5 0
      test/Optimizer/rlexe.xml

+ 30 - 0
lib/Backend/GlobOptIntBounds.cpp

@@ -1016,6 +1016,36 @@ void GlobOpt::MergeBoundCheckHoistBlockData(
                     mergedInductionVariables->Add(backEdgeInductionVariable);
                 }
             }
+
+            const InductionVariableSet *const fromDataInductionVariables = fromData->inductionVariables;
+            for (auto it = mergedInductionVariables->GetIterator(); it.IsValid(); it.MoveNext())
+            {
+                InductionVariable &mergedInductionVariable = it.CurrentValueReference();
+                if (!mergedInductionVariable.IsChangeDeterminate())
+                {
+                    continue;
+                }
+
+                StackSym *const sym = mergedInductionVariable.Sym();
+                const InductionVariable *fromDataInductionVariable;
+                if (fromDataInductionVariables->TryGetReference(sym->m_id, &fromDataInductionVariable))
+                {
+                    continue;
+                }
+
+                // Process the set of symbols that are induction variables due to prior loops that share the same parent loop, but are not induction variables in the current loop
+                // If the current loop is initializing such carried over induction variables, then their value numbers will differ from the current loop's landing pad
+                // Such induction variables should be marked as indeterminate going forward, such the  induction variable analysis accurately flows to the parent loop.
+                Value *const fromDataValue = fromData->FindValue(sym);
+                if (fromDataValue)
+                {
+                    Value *const landingPadValue = toBlock->loop->landingPad->globOptData.FindValue(sym);
+                    if (landingPadValue && fromDataValue->GetValueNumber() != landingPadValue->GetValueNumber())
+                    {
+                        mergedInductionVariable.SetChangeIsIndeterminate();
+                    }
+                }
+            }
             return;
         }
 

+ 41 - 0
test/Optimizer/invalidIVRangeBug.js

@@ -0,0 +1,41 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+function test0() {
+  var GiantPrintArray = [];
+  var obj0 = {};
+  var IntArr0 = [];
+  var IntArr1 = [];
+  var VarArr0 = [obj0];
+  var e = -649211448;
+  var f = 137044716;
+  var protoObj1 = Object();
+  function v0(v1) {
+    var v4 = {};
+    v4.a = v1;
+    v4.a[1] = null;
+  }
+  GiantPrintArray.push(v0(IntArr0));
+  for (var _strvar26 in VarArr0) {
+    for (; IntArr1.push(); f++) {
+    }
+    for (var _strvar0 in IntArr0) {
+      f = (e > _strvar0 & 255);
+    }
+  }
+  protoObj1.prop5 = { prop3: !f };
+  return protoObj1.prop5.prop3;
+}
+
+var x = test0();
+x &= test0();
+x &= test0();
+
+if (x == true) {
+  WScript.Echo("PASSED");
+}
+else {
+  WScript.Echo("FAILED");
+}

+ 5 - 0
test/Optimizer/rlexe.xml

@@ -1424,4 +1424,9 @@
       <compile-flags>-maxinterpretcount:1 -maxsimplejitruncount:1</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>invalidIVRangeBug.js</files>
+    </default>
+  </test>
 </regress-exe>