Skip to content

Commit 5e5e7b1

Browse files
PenguinwizzardPenguinwizzard
committed
[MERGE chakra-core#5167 @Penguinwizzard] Fix a couple issues with hoisting speculation guards
Merge pull request chakra-core#5167 from Penguinwizzard:ponch_opt_fixes This is a bundle of three fixes that fix a larger set of issues related to proper flow graph and block structure, relating to the insertion of blocks for loop out-edge symbol speculation guards, as well as a fix for some assumptions made when creating or updating those guard instructions. Fixes: OS:17527460 OS:17527515 OS:17527532 OS:17527509 OS:17530964 OS:17528277 OS:17527486 OS:17528710 OS:17528345 OS:17541268
2 parents af0de1c + bc38c65 commit 5e5e7b1

10 files changed

Lines changed: 267 additions & 24 deletions

File tree

‎lib/Backend/BackwardPass.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3264,9 +3264,11 @@ BackwardPass::ProcessBlock(BasicBlock * block)
32643264
loop->symClusterList->MapSet<BVSparse<JitArenaAllocator>*>(symID, [](SymID a, BVSparse<JitArenaAllocator> *symbols) {
32653265
symbols->Set(a);
32663266
}, syms);
3267+
SymTable* symTable = loop->GetFunc()->m_symTable;
32673268
FOREACH_BITSET_IN_SPARSEBV(curSymID, syms)
32683269
{
3269-
if (!loop->GetFunc()->m_symTable->Find(curSymID)->IsStackSym())
3270+
Sym* potentialSym = symTable->Find(curSymID);
3271+
if (potentialSym == nullptr || !potentialSym->IsStackSym())
32703272
{
32713273
syms->Clear(curSymID);
32723274
}
@@ -3281,7 +3283,7 @@ BackwardPass::ProcessBlock(BasicBlock * block)
32813283
FOREACH_SLIST_ENTRY(IR::ByteCodeUsesInstr*, bcuInstr, loop->outwardSpeculationMaskInstrs)
32823284
{
32833285
// Get the upwardExposed information for the previous block
3284-
IR::LabelInstr *blockLabel = bcuInstr->m_prev->AsLabelInstr();
3286+
IR::LabelInstr *blockLabel = bcuInstr->GetBlockStartInstr()->AsLabelInstr();
32853287
BasicBlock* maskingBlock = blockLabel->GetBasicBlock();
32863288
// Since it's possible we have a multi-level loop structure (each with its own mask
32873289
// instructions and dereferenced symbol list), we may be able to avoid masking some
@@ -3667,7 +3669,7 @@ BackwardPass::ProcessBlock(BasicBlock * block)
36673669

36683670
// We need to increment the data use count since we're changing a successor.
36693671
blockPred->IncrementDataUseCount();
3670-
BasicBlock *newBlock = this->func->m_fg->InsertAirlockBlock(this->func->m_fg->FindEdge(blockPred, block));
3672+
BasicBlock *newBlock = this->func->m_fg->InsertAirlockBlock(this->func->m_fg->FindEdge(blockPred, block), true);
36713673
LABELNAMESET(newBlock->GetFirstInstr()->AsLabelInstr(), "Loop out-edge masking block");
36723674
// This is a little bit of a misuse of ByteCodeUsesInstr - we're using it as just
36733675
// a bitvector that we can add things to.

‎lib/Backend/FlowGraph.cpp‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1736,18 +1736,10 @@ FlowGraph::Destroy(void)
17361736
FOREACH_BLOCK(block, this)
17371737
{
17381738
Region * region = block->GetFirstInstr()->AsLabelInstr()->GetRegion();
1739-
Region * predRegion = nullptr;
17401739
FOREACH_PREDECESSOR_BLOCK(predBlock, block)
17411740
{
17421741
BasicBlock* intermediateBlock = block;
1743-
// Skip blocks inserted for airlock/masking purposes
1744-
while ((predBlock->isAirLockBlock || predBlock->isAirLockCompensationBlock) && predBlock->GetFirstInstr()->AsLabelInstr()->GetRegion() == region)
1745-
{
1746-
Assert(predBlock->GetPredList()->HasOne());
1747-
intermediateBlock = predBlock;
1748-
predBlock = predBlock->GetPredList()->Head()->GetPred();
1749-
}
1750-
predRegion = predBlock->GetFirstInstr()->AsLabelInstr()->GetRegion();
1742+
Region * predRegion = predBlock->GetFirstInstr()->AsLabelInstr()->GetRegion();
17511743
if (predBlock->GetLastInstr() == nullptr)
17521744
{
17531745
AssertMsg(region == predRegion, "Bad region propagation through empty block");
@@ -2255,7 +2247,7 @@ FlowGraph::InsertCompBlockToLoopList(Loop *loop, BasicBlock* compBlock, BasicBlo
22552247

22562248
// Insert a block on the given edge
22572249
BasicBlock *
2258-
FlowGraph::InsertAirlockBlock(FlowEdge * edge)
2250+
FlowGraph::InsertAirlockBlock(FlowEdge * edge, bool afterForward /*= false*/)
22592251
{
22602252
BasicBlock * airlockBlock = BasicBlock::New(this);
22612253
BasicBlock * sourceBlock = edge->GetPred();
@@ -2341,7 +2333,12 @@ FlowGraph::InsertAirlockBlock(FlowEdge * edge)
23412333
airlockBlock->SetLastInstr(airlockBr);
23422334

23432335
airlockLabel->SetByteCodeOffset(sinkLabel);
2344-
airlockLabel->SetRegion(sinkLabel->GetRegion());
2336+
2337+
// If we have regions in play, we should update them on the airlock block appropriately
2338+
if (afterForward)
2339+
{
2340+
airlockLabel->SetRegion(sinkLabel->GetRegion());
2341+
}
23452342

23462343
// Fixup flow out of sourceBlock
23472344
IR::BranchInstr *sourceBr = sourceLastInstr->AsBranchInstr();
@@ -2362,7 +2359,7 @@ FlowGraph::InsertAirlockBlock(FlowEdge * edge)
23622359
FlowEdge *dstEdge = this->FindEdge(sinkPrevBlock, sinkBlock);
23632360
if (dstEdge) // Possibility that sourceblock may be same as sinkPrevBlock
23642361
{
2365-
BasicBlock* compensationBlock = this->InsertCompensationCodeForBlockMove(dstEdge, true /*insert comp block to loop list*/, true);
2362+
BasicBlock* compensationBlock = this->InsertCompensationCodeForBlockMove(dstEdge, true /*insert comp block to loop list*/, true, afterForward);
23662363
compensationBlock->IncrementDataUseCount();
23672364
// We need to skip airlock compensation block in globopt as its inserted while globopt is iteration over the blocks.
23682365
compensationBlock->isAirLockCompensationBlock = true;
@@ -2379,7 +2376,7 @@ FlowGraph::InsertAirlockBlock(FlowEdge * edge)
23792376

23802377
// Insert a block on the given edge
23812378
BasicBlock *
2382-
FlowGraph::InsertCompensationCodeForBlockMove(FlowEdge * edge, bool insertToLoopList, bool sinkBlockLoop)
2379+
FlowGraph::InsertCompensationCodeForBlockMove(FlowEdge * edge, bool insertToLoopList /*=false*/, bool sinkBlockLoop /*=false*/, bool afterForward /*=false*/)
23832380
{
23842381
BasicBlock * compBlock = BasicBlock::New(this);
23852382
BasicBlock * sourceBlock = edge->GetPred();
@@ -2463,7 +2460,6 @@ FlowGraph::InsertCompensationCodeForBlockMove(FlowEdge * edge, bool insertToLoo
24632460
compBlock->SetLastInstr(compBr);
24642461

24652462
compLabel->SetByteCodeOffset(sinkLabel);
2466-
compLabel->SetRegion(sinkLabel->GetRegion());
24672463

24682464
// Fixup flow out of sourceBlock
24692465
if (sourceLastInstr->IsBranchInstr())
@@ -2477,13 +2473,20 @@ FlowGraph::InsertCompensationCodeForBlockMove(FlowEdge * edge, bool insertToLoo
24772473
}
24782474
}
24792475

2480-
bool assignRegionsBeforeGlobopt = this->func->HasTry() && (this->func->DoOptimizeTry() ||
2481-
(this->func->IsSimpleJit() && this->func->hasBailout) ||
2482-
this->func->IsLoopBodyInTryFinally());
2476+
if (!afterForward)
2477+
{
2478+
bool assignRegionsBeforeGlobopt = this->func->HasTry() && (this->func->DoOptimizeTry() ||
2479+
(this->func->IsSimpleJit() && this->func->hasBailout) ||
2480+
this->func->IsLoopBodyInTryFinally());
24832481

2484-
if (assignRegionsBeforeGlobopt)
2482+
if (assignRegionsBeforeGlobopt)
2483+
{
2484+
UpdateRegionForBlockFromEHPred(compBlock);
2485+
}
2486+
}
2487+
else
24852488
{
2486-
UpdateRegionForBlockFromEHPred(compBlock);
2489+
compLabel->SetRegion(sinkLabel->GetRegion());
24872490
}
24882491

24892492
return compBlock;

‎lib/Backend/FlowGraph.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,10 @@ class FlowGraph
157157
FlowEdge * AddEdge(BasicBlock * predBlock, BasicBlock * succBlock);
158158
BasicBlock * InsertCompensationCodeForBlockMove(FlowEdge * edge, // Edge where compensation code needs to be inserted
159159
bool insertCompensationBlockToLoopList = false,
160-
bool sinkBlockLoop = false // Loop to which compensation block belongs
160+
bool sinkBlockLoop = false, // Loop to which compensation block belongs
161+
bool afterForward = false // Inserting compentation code after forward pass
161162
);
162-
BasicBlock * InsertAirlockBlock(FlowEdge * edge);
163+
BasicBlock * InsertAirlockBlock(FlowEdge * edge, bool afterForward = false);
163164
void InsertCompBlockToLoopList(Loop *loop, BasicBlock* compBlock, BasicBlock* targetBlock, bool postTarget);
164165
void RemoveUnreachableBlocks();
165166
bool RemoveUnreachableBlock(BasicBlock *block, GlobOpt * globOpt = nullptr);

‎lib/Backend/IR.cpp‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2852,6 +2852,42 @@ Instr::GetPrevRealInstrOrLabel() const
28522852
return instr;
28532853
}
28542854

2855+
///----------------------------------------------------------------------------
2856+
///
2857+
/// Instr::GetPrevLabelInstr
2858+
///
2859+
///----------------------------------------------------------------------------
2860+
IR::LabelInstr *
2861+
Instr::GetPrevLabelInstr() const
2862+
{
2863+
IR::Instr *instr = this->m_prev;
2864+
2865+
while (!instr->IsLabelInstr())
2866+
{
2867+
instr = instr->m_prev;
2868+
AssertMsg(instr, "GetPrevLabelInstr() failed...");
2869+
}
2870+
return instr->AsLabelInstr();
2871+
}
2872+
2873+
///----------------------------------------------------------------------------
2874+
///
2875+
/// Instr::GetPrevLabelInstr
2876+
///
2877+
///----------------------------------------------------------------------------
2878+
IR::Instr *
2879+
Instr::GetBlockStartInstr() const
2880+
{
2881+
IR::Instr *instr = this->m_prev;
2882+
2883+
while (!instr->StartsBasicBlock())
2884+
{
2885+
instr = instr->m_prev;
2886+
AssertMsg(instr, "GetBlockStartInstr() failed...");
2887+
}
2888+
return instr;
2889+
}
2890+
28552891
///----------------------------------------------------------------------------
28562892
///
28572893
/// Instr::GetInsertBeforeByteCodeUsesInstr

‎lib/Backend/IR.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,8 @@ class Instr
295295
IR::Instr * GetNextBranchOrLabel() const;
296296
IR::Instr * GetPrevRealInstr() const;
297297
IR::Instr * GetPrevRealInstrOrLabel() const;
298+
IR::LabelInstr *GetPrevLabelInstr() const;
299+
IR::Instr * GetBlockStartInstr() const;
298300
IR::Instr * GetInsertBeforeByteCodeUsesInstr();
299301
bool IsByteCodeUsesInstrFor(IR::Instr * instr) const;
300302
IR::LabelInstr *GetOrCreateContinueLabel(const bool isHelper = false);

‎test/FlowGraph/mic1msjrc1.js‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
function test0() {
7+
while (undefined) {
8+
if (undefined) {
9+
continue;
10+
}
11+
try {
12+
continue;
13+
} catch (ex) {
14+
} finally {
15+
break;
16+
}
17+
}
18+
}
19+
test0();
20+
test0();
21+
test0();
22+
23+
24+
function test1() {
25+
try {
26+
LABEL2:
27+
while (h != d) {
28+
try {
29+
continue;
30+
} catch (ex) {
31+
continue;
32+
} finally {
33+
return -1839801917;
34+
}
35+
}
36+
} catch (ex) {
37+
}
38+
}
39+
test1();
40+
test1();
41+
test1();
42+
43+
44+
var _oo1obj = undefined;
45+
function test2() {
46+
var _oo1obj = function () {
47+
var _oo1obj = {
48+
prop1 : []
49+
};
50+
for (; ([])[1]; ) {
51+
}
52+
_oo1obj.f1 = undefined;
53+
}();
54+
}
55+
test2();
56+
test2();
57+
test2();
58+
59+
60+
function test3() {
61+
var IntArr1 = Array(1);
62+
for (var _ of IntArr1) {
63+
if (this || 1) {
64+
return 1;
65+
}
66+
}
67+
}
68+
test3();
69+
test3();
70+
test3();
71+
72+
73+
var ary = Array();
74+
var func4 = function () {
75+
for (var _ of ary) {
76+
if (undefined || func4) {
77+
break;
78+
}
79+
}
80+
};
81+
func4();
82+
func4();
83+
func4();
84+
85+
console.log('PASSED');

‎test/FlowGraph/rlexe.xml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<regress-exe>
3+
<test>
4+
<default>
5+
<files>mic1msjrc1.js</files>
6+
<compile-flags>-mic:1 -msjrc:1 -oopjit- -bgjit- -loopinterpretcount:1</compile-flags>
7+
</default>
8+
</test>
9+
<test>
10+
<default>
11+
<files>weird1.js</files>
12+
<compile-flags>-maxinterpretcount:1 -maxsimplejitruncount:1 -werexceptionsupport -oopjit- -off:bailonnoprofile -off:cachedScope</compile-flags>
13+
</default>
14+
</test>
15+
<test>
16+
<default>
17+
<files>weird2.js</files>
18+
<compile-flags>-maxinterpretcount:1 -maxsimplejitruncount:1 -werexceptionsupport -bgjit- -loopinterpretcount:1 -oopjit- -off:simplejit -force:inline</compile-flags>
19+
</default>
20+
</test>
21+
</regress-exe>

‎test/FlowGraph/weird1.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
function test0() {
7+
var func0 = function () {
8+
(function (loopInvariant) {
9+
eval('');
10+
while (
11+
{
12+
prop0: uic8[loopInvariant + 1 & 1],
13+
prop1: (undefined + 11) * Math.floor(--a) + 'method0'
14+
}
15+
)
16+
{
17+
if(4) {
18+
break;
19+
}
20+
strvar1 = 2147483646;
21+
({
22+
prop2: 0,
23+
prop3: undefined,
24+
prop4: (3 & 255, 2)
25+
});
26+
continue;
27+
return 'somestring1';
28+
return 'somestring2';
29+
}
30+
}());
31+
};
32+
var uic8 = new Uint8ClampedArray();
33+
var a = 1;
34+
var temp = 0;
35+
do {
36+
func0();
37+
} while (temp++ < 4);
38+
}
39+
test0();
40+
test0();
41+
test0();
42+
43+
console.log("PASSED");

‎test/FlowGraph/weird2.js‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
var shouldBailout = false;
7+
function test0() {
8+
var loopInvariant = 4;
9+
var obj1 = {};
10+
var i16 = new Int16Array();
11+
function _array2iterate(_array2tmp) {
12+
for (var _array2i in _array2tmp) {
13+
if (_array2i.indexOf('') == 0) {
14+
}
15+
var __loopvar1 = loopInvariant - 6;
16+
for (; obj1.prop0 < ~(shouldBailout ? (Object.defineProperty(obj1, '', {
17+
get: function () {
18+
},
19+
configurable: true
20+
}), obj1.prop0) : obj1.prop0); obj1++) {
21+
5;
22+
if (obj1) {
23+
}
24+
}
25+
_array2iterate(_array2tmp[_array2i]);
26+
obj1.prop0 = {
27+
prop0: obj1.prop0 >> 'caller',
28+
prop1: i16[53 & 255],
29+
prop2: obj1.prop0 >> '',
30+
prop3: new RegExp('xyz') instanceof (typeof Function == 'function' && Function[Symbol.toStringTag] == 'AsyncFunction' ? Function : Object),
31+
prop4: obj1[shouldBailout ? obj1[8] = 'x' : undefined, 8]
32+
};
33+
}
34+
}
35+
_array2iterate([
36+
[],
37+
[]
38+
]);
39+
}
40+
test0();
41+
test0();
42+
test0();
43+
44+
console.log("PASSED");

‎test/rlexedirs.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,4 +356,10 @@
356356
<tags>require_backend</tags>
357357
</default>
358358
</dir>
359+
<dir>
360+
<default>
361+
<files>FlowGraph</files>
362+
<tags>require_backend</tags>
363+
</default>
364+
</dir>
359365
</regress-exe>

0 commit comments

Comments
 (0)