Skip to content

Commit f54dfc0

Browse files
ThomsonTanThomsonTan
committed
[MERGE chakra-core#2518 @ThomsonTan] Add missing StSlot for non-temp registers for JITLoopBody
Merge pull request chakra-core#2518 from ThomsonTan:AddMissingStSlotForJITLoopBody In JITLoopBody, we gnerate StSlot for the last instruction we imported from given bytecode instruction. This works fine when one bytecode instruction is mapped to one IR instruction. But in some cases, one bytecode instruction (`NewConcatStrMulti`) could be converted to multiple IR instructions (`Conv_primStr/NewConcatStrMulti/SetConcatStrMultiItem`), then we only check the very last IR instruction to generate StSlot and miss it for previous instructions. This fix keeps the last processed IR instruction and processes all the IR instructions after that in backward order.
2 parents d0f7c83 + 1290a20 commit f54dfc0

3 files changed

Lines changed: 59 additions & 17 deletions

File tree

‎lib/Backend/IRBuilder.cpp‎

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,7 @@ IRBuilder::Build()
670670
JsUtil::BaseDictionary<IR::Instr*, int, JitArenaAllocator> ignoreExBranchInstrToOffsetMap(m_tempAlloc);
671671

672672
Js::LayoutSize layoutSize;
673+
IR::Instr* lastProcessedInstrForJITLoopBody = m_func->m_headInstr;
673674
for (Js::OpCode newOpcode = m_jnReader.ReadOp(layoutSize); (uint)m_jnReader.GetCurrentOffset() <= lastOffset; newOpcode = m_jnReader.ReadOp(layoutSize))
674675
{
675676
Assert(newOpcode != Js::OpCode::EndOfBlock);
@@ -750,28 +751,43 @@ IRBuilder::Build()
750751
}
751752
}
752753
#endif
753-
if (IsLoopBodyInTry() && m_lastInstr->GetDst() && m_lastInstr->GetDst()->IsRegOpnd() && m_lastInstr->GetDst()->GetStackSym()->HasByteCodeRegSlot())
754+
755+
if (IsLoopBodyInTry() && lastProcessedInstrForJITLoopBody != m_lastInstr)
754756
{
755-
StackSym * dstSym = m_lastInstr->GetDst()->GetStackSym();
756-
Js::RegSlot dstRegSlot = dstSym->GetByteCodeRegSlot();
757-
if (!this->RegIsTemp(dstRegSlot) && !this->RegIsConstant(dstRegSlot))
757+
// traverse in backward so we get new/later value of given symId for storing instead of the earlier/stale
758+
// symId value. m_stSlots is used to prevent multiple stores to the same symId.
759+
FOREACH_INSTR_BACKWARD_EDITING_IN_RANGE(
760+
instr,
761+
instrPrev,
762+
m_lastInstr,
763+
lastProcessedInstrForJITLoopBody->m_next)
758764
{
759-
SymID symId = dstSym->m_id;
760-
if (this->m_stSlots->Test(symId))
765+
if (instr->GetDst() && instr->GetDst()->IsRegOpnd() && instr->GetDst()->GetStackSym()->HasByteCodeRegSlot())
761766
{
762-
// For jitted loop bodies that are in a try block, we consider any symbol that has a
763-
// non-temp bytecode reg slot, to be write-through. Hence, generating StSlots at all
764-
// defs for such symbols
765-
IR::Instr * stSlot = this->GenerateLoopBodyStSlot(dstRegSlot);
766-
AddInstr(stSlot, Js::Constants::NoByteCodeOffset);
767+
StackSym * dstSym = instr->GetDst()->GetStackSym();
768+
Js::RegSlot dstRegSlot = dstSym->GetByteCodeRegSlot();
769+
if (!this->RegIsTemp(dstRegSlot) && !this->RegIsConstant(dstRegSlot))
770+
{
771+
SymID symId = dstSym->m_id;
772+
if (this->m_stSlots->Test(symId))
773+
{
774+
// For jitted loop bodies that are in a try block, we consider any symbol that has a
775+
// non-temp bytecode reg slot, to be write-through. Hence, generating StSlots at all
776+
// defs for such symbols
777+
IR::Instr * stSlot = this->GenerateLoopBodyStSlot(dstRegSlot);
778+
AddInstr(stSlot, Js::Constants::NoByteCodeOffset);
767779

768-
this->m_stSlots->Clear(symId);
769-
}
770-
else
771-
{
772-
Assert(dstSym->m_isCatchObjectSym);
780+
this->m_stSlots->Clear(symId);
781+
}
782+
else
783+
{
784+
Assert(dstSym->m_isCatchObjectSym);
785+
}
786+
}
773787
}
774-
}
788+
} NEXT_INSTR_BACKWARD_EDITING_IN_RANGE;
789+
790+
lastProcessedInstrForJITLoopBody = m_lastInstr;
775791
}
776792

777793
offset = m_jnReader.GetCurrentOffset();

‎test/Bugs/MissToGenerateStStSlotForJITLoopBody.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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(i)
7+
{
8+
try{
9+
for (var j = 0; j < 20010; j++) {
10+
if (j > 20000)
11+
return 'j ' + i + ' j in the loop ' + i;
12+
else if (j > 30000)
13+
return ' test0 ' + j;
14+
}
15+
} catch(e){}
16+
}
17+
18+
var ret = test0() + ' test StSlot generation';
19+
20+
print('pass');

‎test/Bugs/rlexe.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,4 +380,10 @@
380380
<compile-flags>-maxinterpretcount:1 -off:simplejit</compile-flags>
381381
</default>
382382
</test>
383+
<test>
384+
<default>
385+
<files>MissToGenerateStStSlotForJITLoopBody.js</files>
386+
<compile-flags>-mic:1 -off:simplejit -oopjit- -bgjit-</compile-flags>
387+
</default>
388+
</test>
383389
</regress-exe>

0 commit comments

Comments
 (0)