Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions lib/Backend/GlobOptBailOut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,43 @@ GlobOpt::ConvertToByteCodeUses(IR::Instr * instr)
this->CaptureByteCodeSymUses(instr);
IR::ByteCodeUsesInstr * byteCodeUsesInstr = this->InsertByteCodeUses(instr, true);
instr->Remove();
// If possible, we want to aggregate with subsequent ByteCodeUses Instructions, so
// that we can do some optimizations in other places where we can simplify args in
// a compare, but still need to generate them for bailouts. Without this, we cause
// problems because we end up with an instruction losing atomicity in terms of its
// bytecode use and generation lifetimes.
if (byteCodeUsesInstr &&
byteCodeUsesInstr->m_next &&
byteCodeUsesInstr->m_next->m_opcode == Js::OpCode::ByteCodeUses &&
byteCodeUsesInstr->GetByteCodeOffset() == byteCodeUsesInstr->m_next->GetByteCodeOffset()
)
{
IR::ByteCodeUsesInstr * nextinstr = byteCodeUsesInstr->m_next->AsByteCodeUsesInstr();
if (nextinstr->GetDst() == nullptr)
{
// We move all of the uses of the next bytecodeuses instruction into this one. The
// instruction is notably not removed or repurposed; unfortunately, at this point,
// doing either would be somewhat complex. Removing the instruction breaks some of
// the assumptions made in other code (notably RemoveCodeAfterNoFallthroughInstr),
// and repurposing it by removing the newly-added byteCodeUsesInstr requires us to
// be able to handle transferring the Dst reg in a couple more cases (like if it's
// single-def).
if (nextinstr->byteCodeUpwardExposedUsed)
{
if (byteCodeUsesInstr->byteCodeUpwardExposedUsed)
{
byteCodeUsesInstr->byteCodeUpwardExposedUsed->Or(nextinstr->byteCodeUpwardExposedUsed);
JitAdelete(nextinstr->byteCodeUpwardExposedUsed->GetAllocator(), nextinstr->byteCodeUpwardExposedUsed);
nextinstr->byteCodeUpwardExposedUsed = nullptr;
}
else
{
byteCodeUsesInstr->byteCodeUpwardExposedUsed = nextinstr->byteCodeUpwardExposedUsed;
nextinstr->byteCodeUpwardExposedUsed = nullptr;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not delete nextinstr at this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code in RemoveCodeAfterNoFallthroughInstr has already referenced the next instruction by the time that we do this. Doing nextinstr->Remove() causes an access violation a couple frames up the call stack if you remove the instruction here.

}
}
return byteCodeUsesInstr;
}

Expand Down
15 changes: 15 additions & 0 deletions test/Bugs/bug9080773_2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//-------------------------------------------------------------------------------------------------------
// 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 protoObj1 = {};
for (var __loopSecondaryVar1000_0 = 10; protoObj1.length; protoObj1) {
var v4 = prop0 >>> 0 < 0;
}
}
test0();
test0();
test0();

WScript.Echo("PASSED");
6 changes: 6 additions & 0 deletions test/Bugs/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,12 @@
<compile-flags>-maxinterpretcount:1 -off:simplejit</compile-flags>
</default>
</test>
<test>
<default>
<files>bug9080773_2.js</files>
<compile-flags>-maxinterpretcount:1 -maxsimplejitruncount:1</compile-flags>
</default>
</test>
<test>
<default>
<files>b208_asmjs.js</files>
Expand Down