Skip to content

Commit 6a9b83b

Browse files
sigatrevsigatrev
committed
[MERGE #5716 @sigatrev] fix two bailout issues with callback.call inlining
Merge pull request #5716 from sigatrev:callback 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.
2 parents 50e23bd + c6278e2 commit 6a9b83b

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
@@ -2926,11 +2926,7 @@ bool Inline::InlineApplyScriptTarget(IR::Instr *callInstr, const FunctionJITTime
29262926
bool originalCallTargetOpndIsJITOpt = callInstr->GetSrc1()->GetIsJITOptimizedReg();
29272927
bool safeThis = false;
29282928

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

3260-
if (targetIsCallback)
3261-
{
3262-
if (!isCallInstanceFunction)
3263-
{
3264-
callInstr->ReplaceSrc1(GetCallbackFunctionOpnd(callInstr));
3265-
}
3266-
}
3267-
else if (!TryGetFixedMethodsForBuiltInAndTarget(callInstr, inlinerData, inlineeData, callFuncInfo, callLdInstr, callTargetLdInstr, safeThis, /*isApplyTarget*/ false))
3256+
if (!TryGetFixedMethodsForBuiltInAndTarget(callInstr, inlinerData, inlineeData, callFuncInfo, callLdInstr, callTargetLdInstr, safeThis, /*isApplyTarget*/ false, targetIsCallback))
32683257
{
32693258
return false;
32703259
}
@@ -3395,7 +3384,7 @@ Inline::SkipCallApplyScriptTargetInlining_Shared(IR::Instr *callInstr, const Fun
33953384

33963385
bool
33973386
Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const FunctionJITTimeInfo* inlinerData, const FunctionJITTimeInfo* inlineeData, const FunctionJITTimeInfo *builtInFuncInfo,
3398-
IR::Instr* builtInLdInstr, IR::Instr* targetLdInstr, bool& safeThis, bool isApplyTarget)
3387+
IR::Instr* builtInLdInstr, IR::Instr* targetLdInstr, bool& safeThis, bool isApplyTarget, bool isCallback)
33993388
{
34003389
#if ENABLE_DEBUG_CONFIG_OPTIONS
34013390
char16 debugStringBuffer[MAX_FUNCTION_BODY_DEBUG_STRING_SIZE];
@@ -3411,6 +3400,29 @@ Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const Functi
34113400

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

3403+
if (isCallback)
3404+
{
3405+
IR::Opnd * functionOpnd = GetCallbackFunctionOpnd(callInstr);
3406+
3407+
// Emit Fixed Method check for apply/call
3408+
safeThis = false;
3409+
if (!TryOptimizeCallInstrWithFixedMethod(callInstr, builtInFuncInfo/*funcinfo for apply/call */, false /*isPolymorphic*/, true /*isBuiltIn*/, false /*isCtor*/, true /*isInlined*/, safeThis /*unused here*/))
3410+
{
3411+
callInstr->ReplaceSrc1(builtInLdInstr->GetDst());
3412+
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"),
3413+
inlineeData->GetBody()->GetDisplayName(), inlineeData->GetDebugNumberSet(debugStringBuffer),
3414+
inlinerData->GetBody()->GetDisplayName(), inlinerData->GetDebugNumberSet(debugStringBuffer2),
3415+
this->topFunc->GetJITFunctionBody()->GetDisplayName(), this->topFunc->GetDebugNumberSet(debugStringBuffer3));
3416+
return false;
3417+
}
3418+
callInstr->m_opcode = originalCallOpCode;
3419+
callInstr->ReplaceSrc1(functionOpnd);
3420+
3421+
useCallTargetInstr->SetRemovedOpndSymbol(originalCallTargetOpndJITOpt, originalCallTargetStackSym->m_id);
3422+
callInstr->InsertBefore(useCallTargetInstr);
3423+
return true;
3424+
}
3425+
34143426
safeThis = false;
34153427
// Check if we can get fixed method for call
34163428
if (TryOptimizeCallInstrWithFixedMethod(callInstr, builtInFuncInfo/*funcinfo for call*/, false /*isPolymorphic*/, true /*isBuiltIn*/, false /*isCtor*/, true /*isInlined*/,
@@ -3424,7 +3436,7 @@ Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const Functi
34243436
safeThis /*unused here*/, true /*dontOptimizeJustCheck*/))
34253437
{
34263438
callInstr->ReplaceSrc1(builtInLdInstr->GetDst());
3427-
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"),
3439+
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"),
34283440
inlineeData->GetBody()->GetDisplayName(), inlineeData->GetDebugNumberSet(debugStringBuffer),
34293441
inlinerData->GetBody()->GetDisplayName(), inlinerData->GetDebugNumberSet(debugStringBuffer2),
34303442
this->topFunc->GetJITFunctionBody()->GetDisplayName(), this->topFunc->GetDebugNumberSet(debugStringBuffer3));
@@ -3433,7 +3445,7 @@ Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const Functi
34333445
}
34343446
else
34353447
{
3436-
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"),
3448+
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"),
34373449
inlineeData->GetBody()->GetDisplayName(), inlineeData->GetDebugNumberSet(debugStringBuffer),
34383450
inlinerData->GetBody()->GetDisplayName(), inlinerData->GetDebugNumberSet(debugStringBuffer2),
34393451
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)