From cb4e16038d0ee31aea1a85365aafbd620b875154 Mon Sep 17 00:00:00 2001 From: chakrabot Date: Sat, 12 Aug 2017 03:02:45 -0700 Subject: [PATCH] [Merge Microsoft/Chakracore@68cc4e864c] [1.6>1.7] [MERGE #3504 @meg-gupta] Fix IV analysis when a loop modifies an induction variable of another sibling loop sharing the same loop parent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../core/lib/Backend/GlobOptIntBounds.cpp | 30 ++++++++++++++ .../core/test/Optimizer/invalidIVRangeBug.js | 41 +++++++++++++++++++ deps/chakrashim/core/test/Optimizer/rlexe.xml | 5 +++ 3 files changed, 76 insertions(+) create mode 100644 deps/chakrashim/core/test/Optimizer/invalidIVRangeBug.js diff --git a/deps/chakrashim/core/lib/Backend/GlobOptIntBounds.cpp b/deps/chakrashim/core/lib/Backend/GlobOptIntBounds.cpp index 86bfdcf1b27..d6d287daa4d 100644 --- a/deps/chakrashim/core/lib/Backend/GlobOptIntBounds.cpp +++ b/deps/chakrashim/core/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; } diff --git a/deps/chakrashim/core/test/Optimizer/invalidIVRangeBug.js b/deps/chakrashim/core/test/Optimizer/invalidIVRangeBug.js new file mode 100644 index 00000000000..a8ecfedee56 --- /dev/null +++ b/deps/chakrashim/core/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"); +} diff --git a/deps/chakrashim/core/test/Optimizer/rlexe.xml b/deps/chakrashim/core/test/Optimizer/rlexe.xml index 01634faf32a..8bf31fef4fa 100644 --- a/deps/chakrashim/core/test/Optimizer/rlexe.xml +++ b/deps/chakrashim/core/test/Optimizer/rlexe.xml @@ -1424,4 +1424,9 @@ -maxinterpretcount:1 -maxsimplejitruncount:1 + + + invalidIVRangeBug.js + +