Skip to content

Commit 7830e58

Browse files
[MERGE #1657 @Penguinwizzard] Constrain conv_bool optimization to improve correctness.
Merge pull request #1657 from Penguinwizzard:fg_fix Due to my recent change to the unsigned compare peep, the conv_bool optimization was causing instructions to be generated in a way that the some registers could have two overlapping lifetimes. This fixes the issue by checking for these ByteCodeUses instructions, and then placing the conv_bool after them.
2 parents ade87f6 + 713051c commit 7830e58

File tree

3 files changed

+43
-1
lines changed

3 files changed

+43
-1
lines changed

lib/Backend/GlobOpt.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12257,7 +12257,19 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
1225712257

1225812258
varDst = IR::RegOpnd::New(intDst->m_sym->GetVarEquivSym(this->func), TyVar, this->func);
1225912259
IR::Instr *convBoolInstr = IR::Instr::New(Js::OpCode::Conv_Bool, varDst, intDst, this->func);
12260-
instr->InsertAfter(convBoolInstr);
12260+
// In some cases (e.g. unsigned compare peep code), a comparison will use variables
12261+
// other than the ones initially intended for it, if we can determine that we would
12262+
// arrive at the same result. This means that we get a ByteCodeUses operation after
12263+
// the actual comparison. Since Inserting the Conv_bool just after the compare, and
12264+
// just before the ByteCodeUses, would cause issues later on with register lifetime
12265+
// calculation, we want to insert the Conv_bool after the whole compare instruction
12266+
// block.
12267+
IR::Instr *putAfter = instr;
12268+
while (putAfter->m_next && putAfter->m_next->m_opcode == Js::OpCode::ByteCodeUses)
12269+
{
12270+
putAfter = putAfter->m_next;
12271+
}
12272+
putAfter->InsertAfter(convBoolInstr);
1226112273
convBoolInstr->SetByteCodeOffset(instr);
1226212274

1226312275
this->ToVarRegOpnd(varDst, this->currentBlock);

test/Bugs/bug9080773.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
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 func(d0) {
7+
i1 = ((d0) == (+((((((0x517ddbc)-(0xc7276b6f)+(-0x8000000))>>>(((0x0)))) >= (0x4866034e))))));
8+
return i1 + d0;
9+
}
10+
11+
var d1 = (1.015625);
12+
func(d1);
13+
func(d1);
14+
15+
func(d1);
16+
func(d1);
17+
func(d1);
18+
func(d1);
19+
20+
runningJITtedCode = true;
21+
func(d1);
22+
func(d1);
23+
24+
WScript.Echo("PASSED");

test/Bugs/rlexe.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,12 @@
284284
<compile-flags> -bgjit- -simdjs -simd128typespec -mic:1 -lic:1 -off:simplejit</compile-flags>
285285
</default>
286286
</test>
287+
<test>
288+
<default>
289+
<files>bug9080773.js</files>
290+
<compile-flags>-maxinterpretcount:1 -off:simplejit</compile-flags>
291+
</default>
292+
</test>
287293
<test>
288294
<default>
289295
<files>b208_asmjs.js</files>

0 commit comments

Comments
 (0)