Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
[Merge chakra-core/ChakraCore@68cc4e864c] [1.6>1.7] [MERGE #3504 @meg…
Browse files Browse the repository at this point in the history
…-gupta] Fix IV analysis when a loop modifies an induction variable of another sibling loop sharing the same loop parent

Merge pull request #3504 from meg-gupta:fixiv

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 no�t 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.
  • Loading branch information
chakrabot authored and MSLaguana committed Sep 25, 2017
1 parent 3bf590a commit cb4e160
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 0 deletions.
30 changes: 30 additions & 0 deletions deps/chakrashim/core/lib/Backend/GlobOptIntBounds.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
41 changes: 41 additions & 0 deletions deps/chakrashim/core/test/Optimizer/invalidIVRangeBug.js
Original file line number Diff line number Diff line change
@@ -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 changes: 5 additions & 0 deletions deps/chakrashim/core/test/Optimizer/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1424,4 +1424,9 @@
<compile-flags>-maxinterpretcount:1 -maxsimplejitruncount:1</compile-flags>
</default>
</test>
<test>
<default>
<files>invalidIVRangeBug.js</files>
</default>
</test>
</regress-exe>

0 comments on commit cb4e160

Please sign in to comment.