Skip to content

Commit 8fcb0f1

Browse files
author
Kevin Smith
committed
[MERGE #6247 @zenparsing] Insert a ByteCodeUses instr for arguments when inlining apply target
Merge pull request #6247 from zenparsing:inline-apply-target-bug When inline apply target optimization is used for: ```js inlinee.apply(obj, arguments); ``` a bailout will occur if the target function is not the expected inlinee. When this occurs, the arguments object needs to be created for use in the interpreter. Currently, GlobOpt is not correctly tracking that the arguments object symbol is needed by the interpreter, and the arguments object is not getting added to the associated bailout info. This PR adds a `ByteCodeUses` instr for this inlining case. Additional changes: - Explicit initialization for the `m_nonEscapingArgObjAlias` field of StackSym. - `Src2` is no longer used for `BytecodeArgOutUse` (GlobOpt was not checking Src2)
2 parents 9dd45f0 + b4b3b75 commit 8fcb0f1

File tree

4 files changed

+43
-2
lines changed

4 files changed

+43
-2
lines changed

lib/Backend/Inline.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3088,15 +3088,23 @@ bool Inline::InlineApplyScriptTarget(IR::Instr *callInstr, const FunctionJITTime
30883088
// set src1 to avoid CSE on BailOnNotStackArgs for different arguments object
30893089
bailOutOnNotStackArgs->SetSrc1(argumentsObjArgOut->GetSrc1()->Copy(this->topFunc));
30903090
argumentsObjArgOut->InsertBefore(bailOutOnNotStackArgs);
3091+
3092+
// Insert ByteCodeUses instr to ensure that arguments object is available on bailout
3093+
IR::ByteCodeUsesInstr* bytecodeUses = IR::ByteCodeUsesInstr::New(callInstr);
3094+
IR::Opnd* argSrc1 = argObjByteCodeArgoutCapture->GetSrc1();
3095+
bytecodeUses->SetRemovedOpndSymbol(argSrc1->GetIsJITOptimizedReg(), argSrc1->GetStackSym()->m_id);
3096+
callInstr->InsertBefore(bytecodeUses);
30913097
}
30923098

30933099
IR::Instr* byteCodeArgOutUse = IR::Instr::New(Js::OpCode::BytecodeArgOutUse, callInstr->m_func);
30943100
byteCodeArgOutUse->SetSrc1(implicitThisArgOut->GetSrc1());
3101+
callInstr->InsertBefore(byteCodeArgOutUse);
30953102
if (argumentsObjArgOut)
30963103
{
3097-
byteCodeArgOutUse->SetSrc2(argumentsObjArgOut->GetSrc1());
3104+
byteCodeArgOutUse = IR::Instr::New(Js::OpCode::BytecodeArgOutUse, callInstr->m_func);
3105+
byteCodeArgOutUse->SetSrc1(argumentsObjArgOut->GetSrc1());
3106+
callInstr->InsertBefore(byteCodeArgOutUse);
30983107
}
3099-
callInstr->InsertBefore(byteCodeArgOutUse);
31003108

31013109
// don't need the implicit "this" anymore
31023110
explicitThisArgOut->ReplaceSrc2(startCall->GetDst());

lib/Backend/Sym.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ StackSym::New(SymID id, IRType type, Js::RegSlot byteCodeRegSlot, Func *func)
4848
stackSym->m_isTypeSpec = false;
4949
stackSym->m_isArgSlotSym = false;
5050
stackSym->m_isArgSlotRegSym = false;
51+
stackSym->m_nonEscapingArgObjAlias = false;
5152
stackSym->m_isParamSym = false;
5253
stackSym->m_isImplicitParamSym = false;
5354
stackSym->m_isBailOutReferenced = false;

test/inlining/applyBailoutArgs.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
var hasArgs = true;
2+
3+
function method0() {}
4+
5+
function f1(a = (hasArgs = false)) {}
6+
7+
function f2() {
8+
this.method0.apply(this, arguments);
9+
}
10+
11+
function test0() {
12+
var obj1 = {};
13+
obj1.method0 = f1;
14+
obj1.method1 = f2;
15+
obj1.method1.call(undefined);
16+
obj1.method1(1);
17+
}
18+
19+
test0();
20+
test0();
21+
test0();
22+
23+
if (hasArgs) {
24+
WScript.Echo('Passed');
25+
} else {
26+
WScript.Echo('Arguments not passed to inlinee on bailout');
27+
}

test/inlining/rlexe.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@
107107
<compile-flags>-maxinterpretcount:1 -maxsimplejitruncount:0</compile-flags>
108108
</default>
109109
</test>
110+
<test>
111+
<default>
112+
<files>applyBailoutArgs.js</files>
113+
</default>
114+
</test>
110115
<test>
111116
<default>
112117
<files>bugs.js</files>

0 commit comments

Comments
 (0)