Skip to content

Commit da8e8dd

Browse files
sigatrevsigatrev
committed
reinstate LdLen in bytecode with additional profile info
Array optimizations in the backend require precise information about the type of the array being used. The FldInfo profile data associated with LdFld opcodes contains only the type of the value being loaded, so bailouts occuring on array type checks for LdLen didn't always result in updated profiled data, which could lead to infinite bailouts. This commit reintroduces LdLen to the bytecode generator, but changes the way profile data is handled for it. LdLen now has profile data split into 2 places. It has the FldInfo associated with a LdFld instruction to allow object.length to be optimized, but also has a separate LdLenInfo entry containing the type of the object being loaded from. This choice of implementation was driven by the fact that FldInfo in the profile data uses the same index as the InlineCache. LdLen_A is given a OpLayoutDynamicProfile<ElementCP> layout to include both inline cache index and the ProfileId for the LdLenInfo. In the interpreter LdLen_A is handled the same as LdFld except for the addition of recording the object type being loaded from.
1 parent c17731d commit da8e8dd

27 files changed

Lines changed: 369 additions & 174 deletions

‎lib/Backend/GlobOpt.cpp‎

Lines changed: 31 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2346,45 +2346,37 @@ GlobOpt::IsInstrInvalidForMemOp(IR::Instr *instr, Loop *loop, Value *src1Val, Va
23462346
void
23472347
GlobOpt::TryReplaceLdLen(IR::Instr *& instr)
23482348
{
2349-
// Change LdFld on arrays, strings, and 'arguments' to LdLen when we're accessing the .length field
2350-
if ((instr->GetSrc1() && instr->GetSrc1()->IsSymOpnd() && instr->m_opcode == Js::OpCode::ProfiledLdFld) || instr->m_opcode == Js::OpCode::LdFld || instr->m_opcode == Js::OpCode::ScopedLdFld)
2349+
// Change LdLen on objects other than arrays, strings, and 'arguments' to LdFld. Otherwise, convert the SymOpnd to a RegOpnd here.
2350+
if (instr->m_opcode == Js::OpCode::LdLen_A && instr->GetSrc1() && instr->GetSrc1()->IsSymOpnd())
23512351
{
23522352
IR::SymOpnd * opnd = instr->GetSrc1()->AsSymOpnd();
23532353
Sym *sym = opnd->m_sym;
2354-
if (sym->IsPropertySym())
2355-
{
2356-
PropertySym *originalPropertySym = sym->AsPropertySym();
2357-
// only on .length
2358-
if (this->lengthEquivBv != nullptr && this->lengthEquivBv->Test(originalPropertySym->m_id))
2359-
{
2360-
IR::RegOpnd* newopnd = IR::RegOpnd::New(originalPropertySym->m_stackSym, IRType::TyVar, instr->m_func);
2361-
ValueInfo *const objectValueInfo = CurrentBlockData()->FindValue(originalPropertySym->m_stackSym)->GetValueInfo();
2362-
// Only for things we'd emit a fast path for
2363-
if (
2364-
objectValueInfo->IsLikelyAnyArray() ||
2365-
objectValueInfo->HasHadStringTag() ||
2366-
objectValueInfo->IsLikelyString() ||
2367-
newopnd->IsArgumentsObject() ||
2368-
(CurrentBlockData()->argObjSyms && CurrentBlockData()->IsArgumentsOpnd(newopnd))
2369-
)
2370-
{
2371-
// We need to properly transfer over the information from the old operand, which is
2372-
// a SymOpnd, to the new one, which is a RegOpnd. Unfortunately, the types mean the
2373-
// normal copy methods won't work here, so we're going to directly copy data.
2374-
newopnd->SetIsJITOptimizedReg(opnd->GetIsJITOptimizedReg());
2375-
newopnd->SetValueType(objectValueInfo->Type());
2376-
newopnd->SetIsDead(opnd->GetIsDead());
2354+
Assert(sym->IsPropertySym());
2355+
PropertySym *originalPropertySym = sym->AsPropertySym();
23772356

2378-
// Now that we have the operand we need, we can go ahead and make the new instr.
2379-
IR::Instr *newinstr = IR::Instr::New(Js::OpCode::LdLen_A, instr->m_func);
2380-
instr->TransferTo(newinstr);
2381-
newinstr->UnlinkSrc1();
2382-
newinstr->SetSrc1(newopnd);
2383-
instr->InsertAfter(newinstr);
2384-
instr->Remove();
2385-
instr = newinstr;
2386-
}
2387-
}
2357+
IR::RegOpnd* newopnd = IR::RegOpnd::New(originalPropertySym->m_stackSym, IRType::TyVar, instr->m_func);
2358+
ValueInfo *const objectValueInfo = CurrentBlockData()->FindValue(originalPropertySym->m_stackSym)->GetValueInfo();
2359+
// things we'd emit a fast path for
2360+
if (
2361+
objectValueInfo->IsLikelyAnyArray() ||
2362+
objectValueInfo->HasHadStringTag() ||
2363+
objectValueInfo->IsLikelyString() ||
2364+
newopnd->IsArgumentsObject() ||
2365+
(CurrentBlockData()->argObjSyms && CurrentBlockData()->IsArgumentsOpnd(newopnd))
2366+
)
2367+
{
2368+
// We need to properly transfer over the information from the old operand, which is
2369+
// a SymOpnd, to the new one, which is a RegOpnd. Unfortunately, the types mean the
2370+
// normal copy methods won't work here, so we're going to directly copy data.
2371+
newopnd->SetIsJITOptimizedReg(opnd->GetIsJITOptimizedReg());
2372+
newopnd->SetValueType(objectValueInfo->Type());
2373+
newopnd->SetIsDead(opnd->GetIsDead());
2374+
instr->ReplaceSrc1(newopnd);
2375+
}
2376+
else
2377+
{
2378+
// otherwise, change the instruction to an LdFld here.
2379+
instr->m_opcode = Js::OpCode::LdFld;
23882380
}
23892381
}
23902382
}
@@ -2427,7 +2419,7 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
24272419
CurrentBlockData()->KillStateForGeneratorYield();
24282420
}
24292421

2430-
// Change LdFld on arrays, strings, and 'arguments' to LdLen when we're accessing the .length field
2422+
// Change LdLen on objects other than arrays, strings, and 'arguments' to LdFld.
24312423
this->TryReplaceLdLen(instr);
24322424

24332425
// Consider: Do we ever get post-op bailout here, and if so is the FillBailOutInfo call in the right place?
@@ -3443,7 +3435,7 @@ GlobOpt::OptSrc(IR::Opnd *opnd, IR::Instr * *pInstr, Value **indirIndexValRef, I
34433435
case Js::OpCode::LdLen_A:
34443436
if(instr->GetSrc1()->IsRegOpnd() && opnd == instr->GetSrc1())
34453437
{
3446-
profiledArrayType = profiledInstr->u.ldElemInfo->GetArrayType();
3438+
profiledArrayType = profiledInstr->u.LdLenArrayType();
34473439
}
34483440
break;
34493441
}
@@ -5060,7 +5052,7 @@ GlobOpt::ValueNumberDst(IR::Instr **pInstr, Value *src1Val, Value *src2Val)
50605052
case Js::OpCode::LdLen_A:
50615053
if (instr->IsProfiledInstr())
50625054
{
5063-
const ValueType profiledValueType(instr->AsProfiledInstr()->u.ldElemInfo->GetElementType());
5055+
const ValueType profiledValueType(instr->AsProfiledInstr()->u.FldInfo().valueType);
50645056
if(!(profiledValueType.IsLikelyInt() && dst->AsRegOpnd()->m_sym->m_isNotInt))
50655057
{
50665058
return this->NewGenericValue(profiledValueType, dst);
@@ -17074,7 +17066,7 @@ GlobOpt::DoLdLenIntSpec(IR::Instr * const instr, const ValueType baseValueType)
1707417066
if(instr &&
1707517067
instr->IsProfiledInstr() &&
1707617068
(
17077-
!instr->AsProfiledInstr()->u.ldElemInfo->GetElementType().IsLikelyInt() ||
17069+
!instr->AsProfiledInstr()->u.FldInfo().valueType.IsLikelyInt() ||
1707817070
instr->GetDst()->AsRegOpnd()->m_sym->m_isNotInt
1707917071
))
1708017072
{

‎lib/Backend/IR.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -608,13 +608,21 @@ class ProfiledInstr: public Instr
608608
const Js::LdElemInfo * ldElemInfo;
609609
const Js::StElemInfo * stElemInfo;
610610
private:
611-
Js::FldInfo::TSize fldInfoData;
611+
struct
612+
{
613+
Js::FldInfo::TSize fldInfoData;
614+
uint16 arrayType; // used by LdLen
615+
};
612616

613617
public:
614618
Js::FldInfo &FldInfo()
615619
{
616620
return reinterpret_cast<Js::FldInfo &>(fldInfoData);
617621
}
622+
ValueType & LdLenArrayType()
623+
{
624+
return reinterpret_cast<ValueType &>(arrayType);
625+
}
618626
} u;
619627

620628
static const uint InvalidProfileId = (uint)-1;

‎lib/Backend/IRBuilder.cpp‎

Lines changed: 112 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -2033,9 +2033,12 @@ IRBuilder::BuildProfiledReg2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot ds
20332033

20342034
Js::OpCodeUtil::ConvertNonCallOpToNonProfiled(newOpcode);
20352035

2036+
Assert(newOpcode == Js::OpCode::BeginSwitch);
2037+
20362038
IR::RegOpnd * src1Opnd = this->BuildSrcOpnd(srcRegSlot);
20372039
IR::RegOpnd * dstOpnd;
2038-
if(newOpcode == Js::OpCode::BeginSwitch && srcRegSlot == dstRegSlot)
2040+
2041+
if(srcRegSlot == dstRegSlot)
20392042
{
20402043
//if the operands are the same for BeginSwitch, don't build a new operand in IR.
20412044
dstOpnd = src1Opnd;
@@ -2045,51 +2048,12 @@ IRBuilder::BuildProfiledReg2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot ds
20452048
dstOpnd = this->BuildDstOpnd(dstRegSlot);
20462049
}
20472050

2048-
bool isProfiled = true;
2049-
const Js::LdElemInfo *ldElemInfo = nullptr;
2050-
if (newOpcode == Js::OpCode::BeginSwitch)
2051-
{
2052-
m_switchBuilder.BeginSwitch();
2053-
switchFound = true;
2054-
newOpcode = Js::OpCode::Ld_A; // BeginSwitch is originally equivalent to Ld_A
2055-
}
2056-
else
2057-
{
2058-
Assert(newOpcode == Js::OpCode::LdLen_A);
2059-
if(m_func->HasProfileInfo())
2060-
{
2061-
ldElemInfo = m_func->GetReadOnlyProfileInfo()->GetLdElemInfo(profileId);
2062-
ValueType arrayType(ldElemInfo->GetArrayType());
2063-
if(arrayType.IsLikelyNativeArray() &&
2064-
(
2065-
(!(m_func->GetTopFunc()->HasTry() && !m_func->GetTopFunc()->DoOptimizeTry()) && m_func->GetWeakFuncRef() && !m_func->HasArrayInfo()) ||
2066-
m_func->IsJitInDebugMode()
2067-
))
2068-
{
2069-
arrayType = arrayType.SetArrayTypeId(Js::TypeIds_Array);
2070-
2071-
// An opnd's value type will get replaced in the forward phase when it is not fixed. Store the array type in the
2072-
// ProfiledInstr.
2073-
Js::LdElemInfo *const newLdElemInfo = JitAnew(m_func->m_alloc, Js::LdElemInfo, *ldElemInfo);
2074-
newLdElemInfo->arrayType = arrayType;
2075-
ldElemInfo = newLdElemInfo;
2076-
}
2077-
src1Opnd->SetValueType(arrayType);
2078-
2079-
if (m_func->GetTopFunc()->HasTry() && !m_func->GetTopFunc()->DoOptimizeTry())
2080-
{
2081-
isProfiled = false;
2082-
}
2083-
}
2084-
else
2085-
{
2086-
isProfiled = false;
2087-
}
2088-
}
2051+
m_switchBuilder.BeginSwitch();
2052+
switchFound = true;
2053+
newOpcode = Js::OpCode::Ld_A; // BeginSwitch is originally equivalent to Ld_A
20892054

20902055
IR::Instr *instr;
20912056

2092-
20932057
if (m_func->DoSimpleJitDynamicProfile())
20942058
{
20952059
// Since we're in simplejit, we want to keep track of the profileid:
@@ -2098,35 +2062,15 @@ IRBuilder::BuildProfiledReg2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot ds
20982062
profiledInstr->isBeginSwitch = newOpcode == Js::OpCode::Ld_A;
20992063
instr = profiledInstr;
21002064
}
2101-
else if(isProfiled)
2065+
else
21022066
{
21032067
IR::ProfiledInstr *profiledInstr = IR::ProfiledInstr::New(newOpcode, dstOpnd, src1Opnd, m_func);
21042068
instr = profiledInstr;
2105-
2106-
switch (newOpcode) {
2107-
case Js::OpCode::Ld_A:
2108-
profiledInstr->u.FldInfo() = Js::FldInfo();
2109-
break;
2110-
case Js::OpCode::LdLen_A:
2111-
profiledInstr->u.ldElemInfo = ldElemInfo;
2112-
break;
2113-
default:
2114-
Assert(false);
2115-
__assume(false);
2116-
}
2117-
}
2118-
else
2119-
{
2120-
instr = IR::Instr::New(newOpcode, dstOpnd, src1Opnd, m_func);
2069+
profiledInstr->u.FldInfo() = Js::FldInfo();
21212070
}
21222071

21232072
this->AddInstr(instr, offset);
21242073

2125-
if(newOpcode == Js::OpCode::LdLen_A && ldElemInfo && !ldElemInfo->WasProfiled() && DoBailOnNoProfile())
2126-
{
2127-
InsertBailOnNoProfile(instr);
2128-
}
2129-
21302074
if(switchFound && instr->IsProfiledInstr())
21312075
{
21322076
m_switchBuilder.SetProfiledInstruction(instr, profileId);
@@ -4502,6 +4446,109 @@ IRBuilder::BuildElementCP(Js::OpCode newOpcode, uint32 offset, Js::RegSlot insta
45024446
}
45034447
}
45044448

4449+
template <typename SizePolicy>
4450+
void
4451+
IRBuilder::BuildProfiledElementCP(Js::OpCode newOpcode, uint32 offset)
4452+
{
4453+
Assert(OpCodeAttr::HasMultiSizeLayout(newOpcode));
4454+
auto layout = m_jnReader.GetLayout<Js::OpLayoutDynamicProfile<Js::OpLayoutT_ElementCP<SizePolicy>>>();
4455+
4456+
if (!PHASE_OFF(Js::ClosureRegCheckPhase, m_func))
4457+
{
4458+
this->DoClosureRegCheck(layout->Value);
4459+
this->DoClosureRegCheck(layout->Instance);
4460+
}
4461+
4462+
BuildProfiledElementCP(newOpcode, offset, layout->Instance, layout->Value, layout->inlineCacheIndex, layout->profileId);
4463+
}
4464+
4465+
void
4466+
IRBuilder::BuildProfiledElementCP(Js::OpCode newOpcode, uint32 offset, Js::RegSlot instance, Js::RegSlot regSlot, Js::CacheId inlineCacheIndex, Js::ProfileId profileId)
4467+
{
4468+
Assert(OpCodeAttr::HasMultiSizeLayout(newOpcode));
4469+
4470+
bool isProfiled = OpCodeAttr::IsProfiledOp(newOpcode);
4471+
4472+
if (isProfiled)
4473+
{
4474+
Js::OpCodeUtil::ConvertNonCallOpToNonProfiled(newOpcode);
4475+
}
4476+
4477+
Assert(newOpcode == Js::OpCode::LdLen_A);
4478+
4479+
Js::PropertyId propertyId = m_func->GetJITFunctionBody()->GetPropertyIdFromCacheId(inlineCacheIndex);
4480+
IR::SymOpnd * fieldSymOpnd = this->BuildFieldOpnd(newOpcode, instance, propertyId, (Js::PropertyIdIndexType) - 1, PropertyKindData, inlineCacheIndex);
4481+
IR::RegOpnd * dstOpnd = this->BuildDstOpnd(regSlot);
4482+
4483+
ValueType arrayType = ValueType::Uninitialized;
4484+
4485+
if (m_func->HasProfileInfo())
4486+
{
4487+
const Js::LdLenInfo * ldLenInfo = m_func->GetReadOnlyProfileInfo()->GetLdLenInfo(profileId);
4488+
arrayType = (ldLenInfo->GetArrayType());
4489+
if (arrayType.IsLikelyNativeArray() &&
4490+
(
4491+
(!(m_func->GetTopFunc()->HasTry() && !m_func->GetTopFunc()->DoOptimizeTry()) && m_func->GetWeakFuncRef() && !m_func->HasArrayInfo()) ||
4492+
m_func->IsJitInDebugMode()
4493+
))
4494+
{
4495+
// An opnd's value type will get replaced in the forward phase when it is not fixed. Store the array type in the ProfiledInstr.
4496+
arrayType = arrayType.SetArrayTypeId(Js::TypeIds_Array);
4497+
}
4498+
fieldSymOpnd->SetValueType(arrayType);
4499+
4500+
if (m_func->GetTopFunc()->HasTry() && !m_func->GetTopFunc()->DoOptimizeTry())
4501+
{
4502+
isProfiled = false;
4503+
}
4504+
}
4505+
else
4506+
{
4507+
isProfiled = false;
4508+
}
4509+
4510+
bool wasNotProfiled = false;
4511+
IR::Instr *instr = nullptr;
4512+
4513+
if (m_func->DoSimpleJitDynamicProfile())
4514+
{
4515+
// Since we're in simplejit, we want to keep track of the profileid:
4516+
IR::JitProfilingInstr * profiledInstr = IR::JitProfilingInstr::New(newOpcode, dstOpnd, fieldSymOpnd, m_func);
4517+
instr = profiledInstr;
4518+
profiledInstr->profileId = profileId;
4519+
}
4520+
else if (isProfiled)
4521+
{
4522+
IR::ProfiledInstr * profiledInstr = IR::ProfiledInstr::New(newOpcode, dstOpnd, fieldSymOpnd, m_func);
4523+
instr = profiledInstr;
4524+
profiledInstr->u.FldInfo() = *(m_func->GetReadOnlyProfileInfo()->GetFldInfo(inlineCacheIndex));
4525+
profiledInstr->u.LdLenArrayType() = arrayType;
4526+
wasNotProfiled = !profiledInstr->u.FldInfo().WasLdFldProfiled();
4527+
dstOpnd->SetValueType(instr->AsProfiledInstr()->u.FldInfo().valueType);
4528+
#if ENABLE_DEBUG_CONFIG_OPTIONS
4529+
if (Js::Configuration::Global.flags.TestTrace.IsEnabled(Js::DynamicProfilePhase))
4530+
{
4531+
const ValueType valueType(profiledInstr->u.FldInfo().valueType);
4532+
char valueTypeStr[VALUE_TYPE_MAX_STRING_SIZE];
4533+
valueType.ToString(valueTypeStr);
4534+
char16 debugStringBuffer[MAX_FUNCTION_BODY_DEBUG_STRING_SIZE];
4535+
Output::Print(_u("TestTrace function %s (%s) ValueType = %i "), m_func->GetJITFunctionBody()->GetDisplayName(), m_func->GetDebugNumberSet(debugStringBuffer), valueTypeStr);
4536+
instr->DumpTestTrace();
4537+
}
4538+
#endif
4539+
}
4540+
else
4541+
{
4542+
instr = IR::Instr::New(newOpcode, dstOpnd, fieldSymOpnd, m_func);
4543+
}
4544+
4545+
this->AddInstr(instr, offset);
4546+
4547+
if (wasNotProfiled && DoBailOnNoProfile())
4548+
{
4549+
InsertBailOnNoProfile(instr);
4550+
}
4551+
}
45054552

45064553
///----------------------------------------------------------------------------
45074554
///

‎lib/Backend/IRBuilder.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ class IRBuilder
166166
void BuildArgInRest();
167167
void BuildElementP(Js::OpCode newOpcode, uint32 offset, Js::RegSlot regSlot, Js::CacheId inlineCacheIndex);
168168
void BuildElementCP(Js::OpCode newOpcode, uint32 offset, Js::RegSlot instance, Js::RegSlot regSlot, Js::CacheId inlineCacheIndex);
169+
void BuildProfiledElementCP(Js::OpCode newOpcode, uint32 offset, Js::RegSlot instance, Js::RegSlot regSlot, Js::CacheId inlineCacheIndex, Js::ProfileId profileId);
169170
void BuildElementC2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot instanceSlot, Js::RegSlot instance2Slot,
170171
Js::RegSlot regSlot, Js::PropertyIdIndexType propertyIdIndex);
171172
void BuildElementScopedC2(Js::OpCode newOpcode, uint32 offset, Js::RegSlot instance2Slot,

‎lib/Backend/JITTimeProfileInfo.cpp‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,27 @@ JITTimeProfileInfo::InitializeJITProfileData(
2828
CompileAssert(sizeof(LdElemIDL) == sizeof(Js::LdElemInfo));
2929
CompileAssert(sizeof(StElemIDL) == sizeof(Js::StElemInfo));
3030

31+
data->profiledLdLenCount = functionBody->GetProfiledLdLenCount();
3132
data->profiledLdElemCount = functionBody->GetProfiledLdElemCount();
3233
data->profiledStElemCount = functionBody->GetProfiledStElemCount();
3334

3435
if (JITManager::GetJITManager()->IsOOPJITEnabled() || isForegroundJIT)
3536
{
37+
data->ldLenData = (LdLenIDL*)profileInfo->GetLdLenInfo();
3638
data->ldElemData = (LdElemIDL*)profileInfo->GetLdElemInfo();
3739
data->stElemData = (StElemIDL*)profileInfo->GetStElemInfo();
3840
}
3941
else
4042
{
41-
// for in-proc background JIT we need to explicitly copy LdElem and StElem info
43+
// for in-proc background JIT we need to explicitly copy LdLen, LdElem, and StElem info
44+
data->ldLenData = AnewArray(alloc, LdLenIDL, data->profiledLdLenCount);
45+
memcpy_s(
46+
data->ldLenData,
47+
data->profiledLdLenCount * sizeof(LdLenIDL),
48+
profileInfo->GetLdLenInfo(),
49+
functionBody->GetProfiledLdLenCount() * sizeof(Js::LdLenInfo)
50+
);
51+
4252
data->ldElemData = AnewArray(alloc, LdElemIDL, data->profiledLdElemCount);
4353
memcpy_s(
4454
data->ldElemData,
@@ -138,6 +148,12 @@ JITTimeProfileInfo::InitializeJITProfileData(
138148
data->flags |= profileInfo->IsOptimizeTryFinallyDisabled() ? Flags_disableOptimizeTryFinally : 0;
139149
}
140150

151+
const Js::LdLenInfo *
152+
JITTimeProfileInfo::GetLdLenInfo(Js::ProfileId ldLenId) const
153+
{
154+
return &(reinterpret_cast<Js::LdLenInfo*>(m_profileData.ldLenData)[ldLenId]);
155+
}
156+
141157
const Js::LdElemInfo *
142158
JITTimeProfileInfo::GetLdElemInfo(Js::ProfileId ldElemId) const
143159
{

0 commit comments

Comments
 (0)