Skip to content

Commit b9871fd

Browse files
MikeHolmanMikeHolman
committed
fix few bugs with oop jit inlining
1 parent 4cc856b commit b9871fd

File tree

6 files changed

+46
-22
lines changed

6 files changed

+46
-22
lines changed

‎lib/Backend/Inline.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ Inline::Optimize(Func *func, __in_ecount_opt(callerArgOutCount) IR::Instr *calle
234234
break;
235235
}
236236

237-
isBuiltIn = InliningDecider::GetBuiltInInfo(inlineeData->GetLocalFunctionId(), &builtInInlineCandidateOpCode, &builtInReturnType, func->GetScriptContext());
237+
isBuiltIn = InliningDecider::GetBuiltInInfo(inlineeData, &builtInInlineCandidateOpCode, &builtInReturnType);
238238

239239
if(!builtInReturnType.IsUninitialized() && instr->GetDst())
240240
{
@@ -3191,6 +3191,7 @@ Inline::WrapArgsOutWithCoerse(Js::BuiltinFunction builtInId, IR::Instr* callInst
31913191
newInstr = argOutInstr->HoistSrc1(Js::OpCode::Coerse_Str);
31923192
isPreOpBailOutNeeded = true;
31933193
newInstr->GetDst()->SetValueType(ValueType::String);
3194+
Assert(UNREACHED); // TODO: OOP JIT, move string to other proc
31943195
newInstr->SetSrc2(IR::AddrOpnd::New(L"String.prototype.match", IR::AddrOpndKindSz, newInstr->m_func));
31953196
argOutInstr->GetSrc1()->SetValueType(ValueType::String);
31963197
}
@@ -3218,6 +3219,7 @@ Inline::WrapArgsOutWithCoerse(Js::BuiltinFunction builtInId, IR::Instr* callInst
32183219
newInstr = argOutInstr->HoistSrc1(Js::OpCode::Coerse_Str);
32193220
isPreOpBailOutNeeded = true;
32203221
newInstr->GetDst()->SetValueType(ValueType::String);
3222+
Assert(UNREACHED); // TODO: OOP JIT, move string to other proc
32213223
newInstr->SetSrc2(IR::AddrOpnd::New(L"String.prototype.replace", IR::AddrOpndKindSz, newInstr->m_func));
32223224
argOutInstr->GetSrc1()->SetValueType(ValueType::String);
32233225
}

‎lib/Backend/InliningDecider.cpp

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ Js::FunctionInfo *InliningDecider::Inline(Js::FunctionBody *const inliner, Js::F
277277

278278
Js::OpCode builtInInlineCandidateOpCode;
279279
ValueType builtInReturnType;
280-
GetBuiltInInfo(functionInfo, &builtInInlineCandidateOpCode, &builtInReturnType, inliner->GetScriptContext());
280+
GetBuiltInInfo(functionInfo, &builtInInlineCandidateOpCode, &builtInReturnType);
281281

282282
if(builtInInlineCandidateOpCode == 0 && builtInReturnType.IsUninitialized())
283283
{
@@ -299,11 +299,13 @@ Js::FunctionInfo *InliningDecider::Inline(Js::FunctionBody *const inliner, Js::F
299299
return functionInfo;
300300
}
301301

302+
303+
// TODO OOP JIT: add FunctionInfo interface so we can combine these?
304+
/* static */
302305
bool InliningDecider::GetBuiltInInfo(
303-
Js::FunctionInfo *const funcInfo,
306+
const FunctionJITTimeInfo *const funcInfo,
304307
Js::OpCode *const inlineCandidateOpCode,
305-
ValueType *const returnType,
306-
Js::ScriptContext *const scriptContext /* = nullptr*/
308+
ValueType *const returnType
307309
)
308310
{
309311
Assert(funcInfo);
@@ -317,26 +319,42 @@ bool InliningDecider::GetBuiltInInfo(
317319
{
318320
return false;
319321
}
320-
return InliningDecider::GetBuiltInInfo(
322+
return InliningDecider::GetBuiltInInfoCommon(
321323
funcInfo->GetLocalFunctionId(),
322324
inlineCandidateOpCode,
323-
returnType,
324-
scriptContext);
325+
returnType);
325326
}
326327

328+
/* static */
327329
bool InliningDecider::GetBuiltInInfo(
328-
uint localFuncId,
330+
Js::FunctionInfo *const funcInfo,
329331
Js::OpCode *const inlineCandidateOpCode,
330-
ValueType *const returnType,
331-
Js::ScriptContext *const scriptContext /* = nullptr*/
332-
)
332+
ValueType *const returnType
333+
)
333334
{
335+
Assert(funcInfo);
334336
Assert(inlineCandidateOpCode);
335337
Assert(returnType);
336338

337339
*inlineCandidateOpCode = (Js::OpCode)0;
338340
*returnType = ValueType::Uninitialized;
339341

342+
if (funcInfo->HasBody())
343+
{
344+
return false;
345+
}
346+
return InliningDecider::GetBuiltInInfoCommon(
347+
funcInfo->GetLocalFunctionId(),
348+
inlineCandidateOpCode,
349+
returnType);
350+
}
351+
352+
bool InliningDecider::GetBuiltInInfoCommon(
353+
uint localFuncId,
354+
Js::OpCode *const inlineCandidateOpCode,
355+
ValueType *const returnType
356+
)
357+
{
340358
// TODO: consider adding another column to JavascriptBuiltInFunctionList.h/LibraryFunction.h
341359
// and getting helper method from there instead of multiple switch labels. And for return value types too.
342360
switch (localFuncId)

‎lib/Backend/InliningDecider.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,23 +59,26 @@ class InliningDecider
5959

6060

6161
static bool GetBuiltInInfo(
62-
Js::FunctionInfo *const funcInfo,
62+
const FunctionJITTimeInfo *const funcInfo,
6363
Js::OpCode *const inlineCandidateOpCode,
64-
ValueType *const returnType,
65-
Js::ScriptContext *const scriptContext = nullptr);
64+
ValueType *const returnType);
6665

6766
static bool GetBuiltInInfo(
68-
uint localFuncId,
67+
Js::FunctionInfo *const funcInfo,
6968
Js::OpCode *const inlineCandidateOpCode,
70-
ValueType *const returnType,
71-
Js::ScriptContext *const scriptContext = nullptr);
69+
ValueType *const returnType);
7270

7371
#if defined(ENABLE_DEBUG_CONFIG_OPTIONS)
7472
static void TraceInlining(Js::FunctionBody *const inliner, const wchar_t* inlineeName, const wchar_t* inlineeFunctionIdandNumberString, uint inlineeByteCodeCount,
7573
Js::FunctionBody* topFunc, uint inlinedByteCodeCount, Js::FunctionBody *const inlinee, uint callSiteId, uint builtIn = -1);
7674
#endif
7775

7876
private:
77+
static bool GetBuiltInInfoCommon(
78+
uint localFuncId,
79+
Js::OpCode *const inlineCandidateOpCode,
80+
ValueType *const returnType);
81+
7982
static bool IsInlineeLeaf(Js::FunctionBody * const inlinee)
8083
{
8184
return inlinee->HasDynamicProfileInfo()

‎lib/Backend/Lower.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10905,7 +10905,7 @@ Lowerer::LowerBailOnNotBuiltIn(IR::Instr *instr,
1090510905
intptr_t builtInFuncs = m_func->GetScriptContextInfo()->GetBuiltinFunctionsBaseAddr();
1090610906
Js::BuiltinFunction builtInIndex = instr->UnlinkSrc2()->AsIntConstOpnd()->AsInt32();
1090710907

10908-
IR::Opnd *builtIn = IR::MemRefOpnd::New((void*)(builtInFuncs + builtInIndex), TyMachReg, instr->m_func);
10908+
IR::Opnd *builtIn = IR::MemRefOpnd::New((void*)(builtInFuncs + builtInIndex * MachPtr), TyMachReg, instr->m_func);
1090910909

1091010910
#if TESTBUILTINFORNULL
1091110911
IR::LabelInstr * continueAfterTestLabel = IR::LabelInstr::New(Js::OpCode::Label, instr->m_func);

‎lib/Runtime/Base/FunctionBody.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8485,15 +8485,15 @@ namespace Js
84858485

84868486
for (unsigned int i = 0; i < this->inlineeFrameOffsetArrayCount - 1; i++)
84878487
{
8488-
if (offsets[i].offset <= offset && offset < offsets[i + 1].offset)
8488+
if (offsets[i].offset < offset && offsets[i + 1].offset >= offset)
84898489
{
8490-
if (offsets[i].recordOffset == NativeOffsetInlineeFrameRecordOffset::InvalidRecordOffset)
8490+
if (offsets[i+1].recordOffset == NativeOffsetInlineeFrameRecordOffset::InvalidRecordOffset)
84918491
{
84928492
return nullptr;
84938493
}
84948494
else
84958495
{
8496-
return (InlineeFrameRecord*)(this->nativeDataBuffer + offsets[i].recordOffset);
8496+
return (InlineeFrameRecord*)(this->nativeDataBuffer + offsets[i+1].recordOffset);
84978497
}
84988498
}
84998499
}

‎lib/Runtime/Base/ScriptContext.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,7 @@ namespace Js
427427
contextData.nativeFloatArrayTypeAddr = (intptr_t)GetLibrary()->GetNativeFloatArrayType();
428428
contextData.charStringCacheAddr = (intptr_t)&GetLibrary()->GetCharStringCache();
429429
contextData.libraryAddr = (intptr_t)GetLibrary();
430+
contextData.builtinFunctionsBaseAddr = (intptr_t)GetLibrary()->GetBuiltinFunctions();
430431
contextData.sideEffectsAddr = (intptr_t)optimizationOverrides.GetAddressOfSideEffects();
431432
contextData.arraySetElementFastPathVtableAddr = (intptr_t)optimizationOverrides.GetAddressOfArraySetElementFastPathVtable();
432433
contextData.intArraySetElementFastPathVtableAddr = (intptr_t)optimizationOverrides.GetAddressOfIntArraySetElementFastPathVtable();

0 commit comments

Comments
 (0)