Skip to content

Fix for an issue with redeferral and stack func init in jitted code #2322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix for an issue with redeferral and stack func init in jitted code #2322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
538db04
dont jit direct references to a nested function proxy
rajatd Jan 4, 2017
57e5e7e
move argouts next to call instruction for apply inlining
rajatd Jan 4, 2017
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
17 changes: 14 additions & 3 deletions lib/Backend/Inline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2394,7 +2394,7 @@ IR::Instr * Inline::InlineApplyWithArgumentsObject(IR::Instr * callInstr, IR::In
IR::Instr * explicitThisArgOut = nullptr;
IR::Instr * argumentsObjArgOut = nullptr;
uint argOutCount = 0;
this->GetArgInstrsForCallAndApply(callInstr, &implicitThisArgOut, &explicitThisArgOut, &argumentsObjArgOut, argOutCount);
this->GetArgInstrsForCallAndApply(callInstr, &implicitThisArgOut, &explicitThisArgOut, &argumentsObjArgOut, argOutCount, false /*moveArgsCloseToCall*/);

// BailOnNotEqual s4.var ---------------New additional BAILOUT if not stack args or actuals exceed 16 at runtime.
// Bailout: #004e (BailOutOnInlineFunction)
Expand Down Expand Up @@ -2442,6 +2442,14 @@ IR::Instr * Inline::InlineApplyWithArgumentsObject(IR::Instr * callInstr, IR::In
IR::Instr* builtInStartInstr;
InsertInlineeBuiltInStartEndTags(callInstr, 3, &builtInStartInstr); //3 args (implicit this + explicit this + arguments = 3)

// Move argouts close to call. Globopt expects this for arguments object tracking.
IR::Instr* argInsertInstr = builtInStartInstr;
builtInStartInstr->IterateArgInstrs([&](IR::Instr* argInstr) {
argInstr->Move(argInsertInstr);
argInsertInstr = argInstr;
return false;
});

IR::Instr *startCall = IR::Instr::New(Js::OpCode::StartCall, callInstr->m_func);
startCall->SetDst(IR::RegOpnd::New(TyVar, callInstr->m_func));
startCall->SetSrc1(IR::IntConstOpnd::New(2, TyInt32, callInstr->m_func)); //2 args (this pointer & ArgOut_A_From_StackArgs for this direct call to init
Expand Down Expand Up @@ -2580,7 +2588,7 @@ IR::Instr * Inline::InlineApplyWithoutArrayArgument(IR::Instr *callInstr, const
return callInstr;
}

void Inline::GetArgInstrsForCallAndApply(IR::Instr* callInstr, IR::Instr** implicitThisArgOut, IR::Instr** explicitThisArgOut, IR::Instr** argumentsOrArrayArgOut, uint &argOutCount)
void Inline::GetArgInstrsForCallAndApply(IR::Instr* callInstr, IR::Instr** implicitThisArgOut, IR::Instr** explicitThisArgOut, IR::Instr** argumentsOrArrayArgOut, uint &argOutCount, bool moveArgsCloseToCall)
{
IR::Opnd * linkOpnd = callInstr->GetSrc2()->AsSymOpnd();
IR::Instr * argInsertInstr = callInstr;
Expand All @@ -2595,7 +2603,10 @@ void Inline::GetArgInstrsForCallAndApply(IR::Instr* callInstr, IR::Instr** impli
linkOpnd->AsSymOpnd()->m_sym->AsStackSym()->m_allocated = true;
ConvertToInlineBuiltInArgOut(argInstr);

argInstr->Move(argInsertInstr);
if (moveArgsCloseToCall)
{
argInstr->Move(argInsertInstr);
}
argInsertInstr = argInstr;

linkOpnd = argInstr->GetSrc2();
Expand Down
2 changes: 1 addition & 1 deletion lib/Backend/Inline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class Inline
IR::Instr * InlineApplyWithoutArrayArgument(IR::Instr *callInstr, const FunctionJITTimeInfo * applyInfo, const FunctionJITTimeInfo * applyTargetInfo);
bool InlineApplyScriptTarget(IR::Instr *callInstr, const FunctionJITTimeInfo* inlinerData, const FunctionJITTimeInfo** pInlineeData, const FunctionJITTimeInfo * applyFuncInfo,
const StackSym *symThis, IR::Instr ** returnInstr, uint recursiveInlineDepth, bool isArrayOpndArgumentsObject, uint argsCount);
void GetArgInstrsForCallAndApply(IR::Instr* callInstr, IR::Instr** implicitThisArgOut, IR::Instr** explicitThisArgOut, IR::Instr** argumentsOrArrayArgOut, uint &argOutCount);
void GetArgInstrsForCallAndApply(IR::Instr* callInstr, IR::Instr** implicitThisArgOut, IR::Instr** explicitThisArgOut, IR::Instr** argumentsOrArrayArgOut, uint &argOutCount, bool moveArgsCloseToCall = true);

IR::Instr * InlineCall(IR::Instr *callInstr, const FunctionJITTimeInfo * inlineeData, const FunctionJITTimeInfo * inlinerData, const StackSym *symThis, bool* pIsInlined, uint callSiteId, uint recursiveInlineDepth);
bool InlineCallTarget(IR::Instr *callInstr, const FunctionJITTimeInfo* inlinerData, const FunctionJITTimeInfo** pInlineeData, const FunctionJITTimeInfo *callFuncInfo,
Expand Down
87 changes: 26 additions & 61 deletions lib/Backend/Lower.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6579,73 +6579,38 @@ Lowerer::GenerateScriptFunctionInit(IR::RegOpnd * regOpnd, IR::Opnd * vtableAddr
Js::FunctionInfoPtrPtr nestedInfo, IR::Opnd * envOpnd, IR::Instr * insertBeforeInstr, bool isZeroed)
{
Func * func = this->m_func;
IR::Opnd * functionProxyOpnd;
IR::Opnd * functionInfoOpnd = nullptr;
IR::Opnd * typeOpnd = nullptr;
bool doCheckTypeOpnd = true;
if (m_func->IsOOPJIT() || !CONFIG_FLAG(OOPJITMissingOpts) || (*nestedInfo)->IsDeferred())
{
functionInfoOpnd = IR::RegOpnd::New(TyMachPtr, func);
InsertMove(functionInfoOpnd, IR::MemRefOpnd::New(nestedInfo, TyMachPtr, func), insertBeforeInstr);
functionProxyOpnd = IR::RegOpnd::New(TyMachPtr, func);
InsertMove(functionProxyOpnd, IR::IndirOpnd::New(functionInfoOpnd->AsRegOpnd(), Js::FunctionInfo::GetOffsetOfFunctionProxy(), TyMachPtr, func), insertBeforeInstr);
typeOpnd = IR::RegOpnd::New(TyMachPtr, func);
InsertMove(typeOpnd, IR::IndirOpnd::New(functionProxyOpnd->AsRegOpnd(), Js::FunctionProxy::GetOffsetOfDeferredPrototypeType(),
TyMachPtr, func), insertBeforeInstr);
}
else
{
Js::FunctionBody * functionBody = (*nestedInfo)->GetFunctionBody();
functionProxyOpnd = CreateFunctionBodyOpnd(functionBody);
Js::ScriptFunctionType * type = functionBody->GetDeferredPrototypeType();
if (type != nullptr)
{
typeOpnd = IR::AddrOpnd::New(type, IR::AddrOpndKindDynamicType, func);
doCheckTypeOpnd = false;
}
else
{
typeOpnd = IR::RegOpnd::New(TyMachPtr, func);
InsertMove(typeOpnd,
IR::MemRefOpnd::New(((byte *)functionBody) + Js::FunctionProxy::GetOffsetOfDeferredPrototypeType(), TyMachPtr, func),
insertBeforeInstr);
}
}

if (doCheckTypeOpnd)
{
IR::LabelInstr * labelHelper = IR::LabelInstr::New(Js::OpCode::Label, func, true);
InsertTestBranch(typeOpnd, typeOpnd, Js::OpCode::BrEq_A, labelHelper, insertBeforeInstr);
IR::LabelInstr * labelDone = IR::LabelInstr::New(Js::OpCode::Label, func, false);
InsertBranch(Js::OpCode::Br, labelDone, insertBeforeInstr);
insertBeforeInstr->InsertBefore(labelHelper);
m_lowererMD.LoadHelperArgument(insertBeforeInstr, functionProxyOpnd);

IR::Instr * callHelperInstr = IR::Instr::New(Js::OpCode::Call, typeOpnd,
IR::HelperCallOpnd::New(IR::JnHelperMethod::HelperEnsureFunctionProxyDeferredPrototypeType, func), func);
insertBeforeInstr->InsertBefore(callHelperInstr);
m_lowererMD.LowerCall(callHelperInstr, 0);
insertBeforeInstr->InsertBefore(labelDone);
}

IR::Opnd * functionInfoOpnd = IR::RegOpnd::New(TyMachPtr, func);
InsertMove(functionInfoOpnd, IR::MemRefOpnd::New(nestedInfo, TyMachPtr, func), insertBeforeInstr);
IR::Opnd * functionProxyOpnd = IR::RegOpnd::New(TyMachPtr, func);
InsertMove(functionProxyOpnd, IR::IndirOpnd::New(functionInfoOpnd->AsRegOpnd(), Js::FunctionInfo::GetOffsetOfFunctionProxy(), TyMachPtr, func), insertBeforeInstr);
IR::Opnd * typeOpnd = IR::RegOpnd::New(TyMachPtr, func);
InsertMove(typeOpnd, IR::IndirOpnd::New(functionProxyOpnd->AsRegOpnd(), Js::FunctionProxy::GetOffsetOfDeferredPrototypeType(),
TyMachPtr, func), insertBeforeInstr);

IR::LabelInstr * labelHelper = IR::LabelInstr::New(Js::OpCode::Label, func, true);
InsertTestBranch(typeOpnd, typeOpnd, Js::OpCode::BrEq_A, labelHelper, insertBeforeInstr);
IR::LabelInstr * labelDone = IR::LabelInstr::New(Js::OpCode::Label, func, false);
InsertBranch(Js::OpCode::Br, labelDone, insertBeforeInstr);
insertBeforeInstr->InsertBefore(labelHelper);
m_lowererMD.LoadHelperArgument(insertBeforeInstr, functionProxyOpnd);

IR::Instr * callHelperInstr = IR::Instr::New(Js::OpCode::Call, typeOpnd,
IR::HelperCallOpnd::New(IR::JnHelperMethod::HelperEnsureFunctionProxyDeferredPrototypeType, func), func);
insertBeforeInstr->InsertBefore(callHelperInstr);
m_lowererMD.LowerCall(callHelperInstr, 0);
insertBeforeInstr->InsertBefore(labelDone);

GenerateMemInit(regOpnd, 0, vtableAddressOpnd, insertBeforeInstr, isZeroed);
GenerateMemInit(regOpnd, Js::ScriptFunction::GetOffsetOfType(), typeOpnd, insertBeforeInstr, isZeroed);
GenerateMemInitNull(regOpnd, Js::ScriptFunction::GetOffsetOfAuxSlots(), insertBeforeInstr, isZeroed);
GenerateMemInitNull(regOpnd, Js::ScriptFunction::GetOffsetOfObjectArray(), insertBeforeInstr, isZeroed);
GenerateMemInit(regOpnd, Js::ScriptFunction::GetOffsetOfConstructorCache(),
LoadLibraryValueOpnd(insertBeforeInstr, LibraryValue::ValueConstructorCacheDefaultInstance),
insertBeforeInstr, isZeroed);
if (!functionInfoOpnd)
{
if (functionProxyOpnd->IsRegOpnd())
{
functionInfoOpnd = IR::IndirOpnd::New(functionProxyOpnd->AsRegOpnd(), Js::FunctionProxy::GetOffsetOfFunctionInfo(), TyMachReg, func);
}
else
{
functionInfoOpnd = IR::MemRefOpnd::New((BYTE*)functionProxyOpnd->AsAddrOpnd()->m_address + Js::FunctionProxy::GetOffsetOfFunctionInfo(), TyMachReg, func);
}
}
LoadLibraryValueOpnd(insertBeforeInstr, LibraryValue::ValueConstructorCacheDefaultInstance),
insertBeforeInstr, isZeroed);

functionInfoOpnd = IR::IndirOpnd::New(functionProxyOpnd->AsRegOpnd(), Js::FunctionProxy::GetOffsetOfFunctionInfo(), TyMachReg, func);
Copy link
Contributor Author

@rajatd rajatd Jan 5, 2017

Choose a reason for hiding this comment

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

@pleath - i was in two minds whether to directly use the nestedInfo for initializing functionInfo. I'm sure I have asked you this before, but do we expect functionInfo to be different from functionInfo->functionBodyImpl->functionInfo in some cases?

Copy link
Contributor

@pleath pleath Jan 6, 2017

Choose a reason for hiding this comment

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

No. Never. Once we allocate a FunctionInfo for a given user function, it never changes. So, yes, once we've loaded it from the nested array we can just reuse it throughout the code sequence.


GenerateMemInit(regOpnd, Js::ScriptFunction::GetOffsetOfFunctionInfo(), functionInfoOpnd, insertBeforeInstr, isZeroed);
GenerateMemInit(regOpnd, Js::ScriptFunction::GetOffsetOfEnvironment(), envOpnd, insertBeforeInstr, isZeroed);
GenerateMemInitNull(regOpnd, Js::ScriptFunction::GetOffsetOfCachedScopeObj(), insertBeforeInstr, isZeroed);
Expand Down