Skip to content

Commit c6278e2

Browse files
sigatrevsigatrev
committed
fix two bailout issues with callback.call inlining
First, there was no check to ensure .call/.apply were the real .call/.apply. There is now a CheckFixedFld for loading .call/.apply. Second, when bailing out on the callback function not equal to what was inlined, the register for the .call/.apply method was not being restored.
1 parent 565eee0 commit c6278e2

File tree

4 files changed

+80
-17
lines changed

4 files changed

+80
-17
lines changed

‎lib/Backend/Inline.cpp

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2925,11 +2925,7 @@ bool Inline::InlineApplyScriptTarget(IR::Instr *callInstr, const FunctionJITTime
29252925
bool originalCallTargetOpndIsJITOpt = callInstr->GetSrc1()->GetIsJITOptimizedReg();
29262926
bool safeThis = false;
29272927

2928-
if (targetIsCallback)
2929-
{
2930-
callInstr->ReplaceSrc1(GetCallbackFunctionOpnd(callInstr));
2931-
}
2932-
else if (!TryGetFixedMethodsForBuiltInAndTarget(callInstr, inlinerData, inlineeData, applyFuncInfo, applyLdInstr, applyTargetLdInstr, safeThis, /*isApplyTarget*/ true))
2928+
if (!TryGetFixedMethodsForBuiltInAndTarget(callInstr, inlinerData, inlineeData, applyFuncInfo, applyLdInstr, applyTargetLdInstr, safeThis, /*isApplyTarget*/ true, targetIsCallback))
29332929
{
29342930
return false;
29352931
}
@@ -3256,14 +3252,7 @@ Inline::InlineCallTarget(IR::Instr *callInstr, const FunctionJITTimeInfo* inline
32563252
bool originalCallTargetOpndIsJITOpt = callInstr->GetSrc1()->GetIsJITOptimizedReg();
32573253
bool safeThis = false;
32583254

3259-
if (targetIsCallback)
3260-
{
3261-
if (!isCallInstanceFunction)
3262-
{
3263-
callInstr->ReplaceSrc1(GetCallbackFunctionOpnd(callInstr));
3264-
}
3265-
}
3266-
else if (!TryGetFixedMethodsForBuiltInAndTarget(callInstr, inlinerData, inlineeData, callFuncInfo, callLdInstr, callTargetLdInstr, safeThis, /*isApplyTarget*/ false))
3255+
if (!TryGetFixedMethodsForBuiltInAndTarget(callInstr, inlinerData, inlineeData, callFuncInfo, callLdInstr, callTargetLdInstr, safeThis, /*isApplyTarget*/ false, targetIsCallback))
32673256
{
32683257
return false;
32693258
}
@@ -3394,7 +3383,7 @@ Inline::SkipCallApplyScriptTargetInlining_Shared(IR::Instr *callInstr, const Fun
33943383

33953384
bool
33963385
Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const FunctionJITTimeInfo* inlinerData, const FunctionJITTimeInfo* inlineeData, const FunctionJITTimeInfo *builtInFuncInfo,
3397-
IR::Instr* builtInLdInstr, IR::Instr* targetLdInstr, bool& safeThis, bool isApplyTarget)
3386+
IR::Instr* builtInLdInstr, IR::Instr* targetLdInstr, bool& safeThis, bool isApplyTarget, bool isCallback)
33983387
{
33993388
#if ENABLE_DEBUG_CONFIG_OPTIONS
34003389
char16 debugStringBuffer[MAX_FUNCTION_BODY_DEBUG_STRING_SIZE];
@@ -3410,6 +3399,29 @@ Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const Functi
34103399

34113400
IR::ByteCodeUsesInstr * useCallTargetInstr = IR::ByteCodeUsesInstr::New(callInstr);
34123401

3402+
if (isCallback)
3403+
{
3404+
IR::Opnd * functionOpnd = GetCallbackFunctionOpnd(callInstr);
3405+
3406+
// Emit Fixed Method check for apply/call
3407+
safeThis = false;
3408+
if (!TryOptimizeCallInstrWithFixedMethod(callInstr, builtInFuncInfo/*funcinfo for apply/call */, false /*isPolymorphic*/, true /*isBuiltIn*/, false /*isCtor*/, true /*isInlined*/, safeThis /*unused here*/))
3409+
{
3410+
callInstr->ReplaceSrc1(builtInLdInstr->GetDst());
3411+
INLINE_CALLBACKS_TRACE(_u("INLINING: Skip Inline: Skipping callback.%s target inlining, did not get fixed method for %s \tInlinee: %s (%s)\tCaller: %s\t(%s) \tTop Func:%s\t(%s)\n"), isApplyTarget ? _u("apply") : _u("call"), isApplyTarget ? _u("apply") : _u("call"),
3412+
inlineeData->GetBody()->GetDisplayName(), inlineeData->GetDebugNumberSet(debugStringBuffer),
3413+
inlinerData->GetBody()->GetDisplayName(), inlinerData->GetDebugNumberSet(debugStringBuffer2),
3414+
this->topFunc->GetJITFunctionBody()->GetDisplayName(), this->topFunc->GetDebugNumberSet(debugStringBuffer3));
3415+
return false;
3416+
}
3417+
callInstr->m_opcode = originalCallOpCode;
3418+
callInstr->ReplaceSrc1(functionOpnd);
3419+
3420+
useCallTargetInstr->SetRemovedOpndSymbol(originalCallTargetOpndJITOpt, originalCallTargetStackSym->m_id);
3421+
callInstr->InsertBefore(useCallTargetInstr);
3422+
return true;
3423+
}
3424+
34133425
safeThis = false;
34143426
// Check if we can get fixed method for call
34153427
if (TryOptimizeCallInstrWithFixedMethod(callInstr, builtInFuncInfo/*funcinfo for call*/, false /*isPolymorphic*/, true /*isBuiltIn*/, false /*isCtor*/, true /*isInlined*/,
@@ -3423,7 +3435,7 @@ Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const Functi
34233435
safeThis /*unused here*/, true /*dontOptimizeJustCheck*/))
34243436
{
34253437
callInstr->ReplaceSrc1(builtInLdInstr->GetDst());
3426-
INLINE_TESTTRACE(_u("INLINING: Skip Inline: Skipping %s target inlining, did not get fixed method for %s target \tInlinee: %s (#%d)\tCaller: %s\t(#%d) \tTop Func:%s\t(#%d)\n"), isApplyTarget ? _u("apply") : _u("call"), isApplyTarget ? _u("apply") : _u("call"),
3438+
INLINE_TESTTRACE(_u("INLINING: Skip Inline: Skipping %s target inlining, did not get fixed method for %s target \tInlinee: %s (%s)\tCaller: %s\t(%s) \tTop Func:%s\t(%s)\n"), isApplyTarget ? _u("apply") : _u("call"), isApplyTarget ? _u("apply") : _u("call"),
34273439
inlineeData->GetBody()->GetDisplayName(), inlineeData->GetDebugNumberSet(debugStringBuffer),
34283440
inlinerData->GetBody()->GetDisplayName(), inlinerData->GetDebugNumberSet(debugStringBuffer2),
34293441
this->topFunc->GetJITFunctionBody()->GetDisplayName(), this->topFunc->GetDebugNumberSet(debugStringBuffer3));
@@ -3432,7 +3444,7 @@ Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const Functi
34323444
}
34333445
else
34343446
{
3435-
INLINE_TESTTRACE(_u("INLINING: Skip Inline: Skipping %s target inlining, did not get fixed method for %s \tInlinee: %s (#%d)\tCaller: %s\t(#%d) \tTop Func:%s\t(#%d)\n"), isApplyTarget ? _u("apply") : _u("call"), isApplyTarget ? _u("apply") : _u("call"),
3447+
INLINE_TESTTRACE(_u("INLINING: Skip Inline: Skipping %s target inlining, did not get fixed method for %s \tInlinee: %s (%s)\tCaller: %s\t(%s) \tTop Func:%s\t(%s)\n"), isApplyTarget ? _u("apply") : _u("call"), isApplyTarget ? _u("apply") : _u("call"),
34363448
inlineeData->GetBody()->GetDisplayName(), inlineeData->GetDebugNumberSet(debugStringBuffer),
34373449
inlinerData->GetBody()->GetDisplayName(), inlinerData->GetDebugNumberSet(debugStringBuffer2),
34383450
this->topFunc->GetJITFunctionBody()->GetDisplayName(), this->topFunc->GetDebugNumberSet(debugStringBuffer3));

‎lib/Backend/Inline.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class Inline
8787
uint inlineCacheIndex, bool safeThis, bool isApplyTarget, bool isCallTarget, IR::Instr * inlineeDefInstr, uint recursiveInlineDepth);
8888
bool SkipCallApplyScriptTargetInlining_Shared(IR::Instr *callInstr, const FunctionJITTimeInfo* inlinerData, const FunctionJITTimeInfo* inlineeData, bool isApplyTarget, bool isCallTarget);
8989
bool TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const FunctionJITTimeInfo* inlinerData, const FunctionJITTimeInfo* inlineeData, const FunctionJITTimeInfo *builtInFuncInfo,
90-
IR::Instr* builtInLdInstr, IR::Instr* targetLdInstr, bool& safeThis, bool isApplyTarget);
90+
IR::Instr* builtInLdInstr, IR::Instr* targetLdInstr, bool& safeThis, bool isApplyTarget, bool isCallback);
9191

9292
IR::Instr * InlineBuiltInFunction(IR::Instr *callInstr, const FunctionJITTimeInfo * inlineeData, Js::OpCode inlineCallOpCode, const FunctionJITTimeInfo * inlinerData, const StackSym *symCallerThis, bool* pIsInlined, uint profileId, uint recursiveInlineDepth);
9393
IR::Instr * InlineFunc(IR::Instr *callInstr, const FunctionJITTimeInfo *const inlineeData, const uint profileId);

‎test/inlining/InlineCallbackCallBailout.baseline

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,21 @@ INLINING CALLBACK : Inlining callback for call/apply target : BailOut ( (#1.1),
33
BailOut: function BailOut, Opcode: BoundCheck Kind: BailOutOnNotNativeArray
44
BailOut: function DispatchCallWithThis, Opcode: InlineeEnd Kind: BailOutOnNotNativeArray
55
BailOut: function DispatchBailout, Opcode: InlineeEnd Kind: BailOutOnNotNativeArray
6+
foo
7+
foo
8+
INLINING : Found callback def instr for call/apply target callback at CallSite: 0 Caller: Dispatch ( (#1.5), #6)
9+
INLINING CALLBACK : Inlining callback for call/apply target : foo ( (#1.4), #5)
10+
foo
11+
BailOut: function Dispatch, Opcode: BailOnNotEqual Kind: BailOutOnInlineFunction
12+
bar
13+
BailOut: function CallDispatch, Opcode: InlineeEnd Kind: BailOutOnInlineFunction
14+
foo
15+
BailOut: function Dispatch, Opcode: CheckFixedFld Kind: BailOutFailedEquivalentFixedFieldTypeCheck
16+
foo
17+
BailOut: function CallDispatch, Opcode: InlineeEnd Kind: BailOutFailedEquivalentFixedFieldTypeCheck
18+
BailOut: function Dispatch, Opcode: CheckFixedFld Kind: BailOutFailedEquivalentFixedFieldTypeCheck
19+
foo
20+
BailOut: function CallDispatch, Opcode: InlineeEnd Kind: BailOutFailedEquivalentFixedFieldTypeCheck
21+
INLINING : Found callback def instr for call/apply target callback at CallSite: 0 Caller: Dispatch ( (#1.5), #6)
22+
INLINING: Skip Inline: Skipping callback.call target inlining, did not get fixed method for call Inlinee: foo ( (#1.4), #5) Caller: Dispatch ( (#1.5), #6) Top Func:CallDispatch ( (#1.6), #7)
23+
foo

‎test/inlining/InlineCallbackCallBailout.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,36 @@ function DispatchBailout(arg)
2222
DispatchBailout([1]);
2323
DispatchBailout([1]);
2424
DispatchBailout([1.1]);
25+
26+
// test bail out from having a different callback function
27+
function foo()
28+
{
29+
WScript.Echo("foo");
30+
};
31+
32+
function Dispatch(callback)
33+
{
34+
callback.call(undefined);
35+
}
36+
37+
function CallDispatch(callback)
38+
{
39+
Dispatch(callback);
40+
}
41+
42+
CallDispatch(foo);
43+
CallDispatch(foo);
44+
CallDispatch(foo);
45+
46+
// BailOutOnInlineFunction
47+
CallDispatch(function() { WScript.Echo("bar"); });
48+
49+
CallDispatch(foo);
50+
51+
// tautological statement makes function.call a non-fixed method. Test CheckFixedFld bailout.
52+
Function.prototype.call = Function.prototype.call;
53+
CallDispatch(foo)
54+
CallDispatch(foo)
55+
56+
// rejit, not inlining callback.call
57+
CallDispatch(foo)

0 commit comments

Comments
 (0)