Skip to content

Commit

Permalink
[MERGE #4727 @sigatrev] Array profile data usage optimzation
Browse files Browse the repository at this point in the history 
Merge pull request #4727 from sigatrev:arrayOpts

This commit changes usage of array profile data in an attempt to better handle certain cases (such as inlined common functions) by using the most specialized (int>float>var) array type profiled when merging profile data, unless the instruction has previous bailed out on NotNativeArray, in which case it will use the least specialized type to avoid infinite bailouts.

This also fixes an infinite bailout in Ares-6 ML benchmark which was introduced by #3169
  • Loading branch information
@sigatrev
sigatrev committed Feb 28, 2018
2 parents 74462bc + 395a879 commit 5ed292c
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 29 deletions.
5 changes: 5 additions & 0 deletions lib/Backend/BailOut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1464,6 +1464,11 @@ BailOutRecord::BailOutHelper(Js::JavascriptCallStackLayout * layout, Js::ScriptF
{
newInstance->OrFlags(Js::InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall);
}
else if (bailOutKind == IR::BailOutOnNotNativeArray)
{
newInstance->OrFlags(Js::InterpreterStackFrameFlags_ProcessingBailOutOnArraySpecialization);
}

if (isInlinee)
{
newInstance->OrFlags(Js::InterpreterStackFrameFlags_FromBailOutInInlinee);
Expand Down
43 changes: 28 additions & 15 deletions lib/Backend/GlobOpt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3406,20 +3406,22 @@ GlobOpt::OptSrc(IR::Opnd *opnd, IR::Instr * *pInstr, Value **indirIndexValRef, I
{
ValueType valueType(val->GetValueInfo()->Type());

// This block uses local profiling data to optimize the case of a native array being passed to a function that fills it with other types. When the function is inlined
// into different call paths which use different types this can cause a perf hit by performing unnecessary array conversions, so only perform this optimization when
// the function is not inlined.
if (valueType.IsLikelyNativeArray() && !valueType.IsObject() && instr->IsProfiledInstr() && !instr->m_func->IsInlined())
// This block uses per-instruction profile information on array types to optimize using the best available profile
// information and to prevent infinite bailouts by ensuring array type information is updated on bailouts.
if (valueType.IsLikelyArray() && !valueType.IsObject() && instr->IsProfiledInstr())
{
// See if we have profile data for the array type
IR::ProfiledInstr *const profiledInstr = instr->AsProfiledInstr();
ValueType profiledArrayType;

bool useAggressiveSpecialization = true;
switch(instr->m_opcode)
{
case Js::OpCode::LdElemI_A:
if(instr->GetSrc1()->IsIndirOpnd() && opnd == instr->GetSrc1()->AsIndirOpnd()->GetBaseOpnd())
{
profiledArrayType = profiledInstr->u.ldElemInfo->GetArrayType();
useAggressiveSpecialization = !profiledInstr->u.ldElemInfo->IsAggressiveSpecializationDisabled();
}
break;

Expand All @@ -3429,23 +3431,36 @@ GlobOpt::OptSrc(IR::Opnd *opnd, IR::Instr * *pInstr, Value **indirIndexValRef, I
if(instr->GetDst()->IsIndirOpnd() && opnd == instr->GetDst()->AsIndirOpnd()->GetBaseOpnd())
{
profiledArrayType = profiledInstr->u.stElemInfo->GetArrayType();
useAggressiveSpecialization = !profiledInstr->u.stElemInfo->IsAggressiveSpecializationDisabled();
}
break;

case Js::OpCode::LdLen_A:
if(instr->GetSrc1()->IsRegOpnd() && opnd == instr->GetSrc1())
{
profiledArrayType = profiledInstr->u.LdLenInfo().GetArrayType();
useAggressiveSpecialization = !profiledInstr->u.LdLenInfo().IsAggressiveSpecializationDisabled();
}
break;
}
if(profiledArrayType.IsLikelyObject() &&
profiledArrayType.GetObjectType() == valueType.GetObjectType() &&
(profiledArrayType.HasVarElements() || (valueType.HasIntElements() && profiledArrayType.HasFloatElements())))

if (profiledArrayType.IsLikelyObject() && profiledArrayType.GetObjectType() == valueType.GetObjectType())
{
// Merge array type we pulled from profile with type propagated by dataflow.
valueType = valueType.Merge(profiledArrayType).SetHasNoMissingValues(valueType.HasNoMissingValues());
ChangeValueType(this->currentBlock, CurrentBlockData()->FindValue(opnd->AsRegOpnd()->m_sym), valueType, false);
// Ideally we want to use the most specialized type seen by this path, but when that causes bailouts use the least specialized type instead.
if (useAggressiveSpecialization && !valueType.IsLikelyNativeIntArray() &&
(profiledArrayType.HasIntElements() || (valueType.HasVarElements() && profiledArrayType.HasFloatElements())))
{
// use the more specialized type profiled by the instruction.
valueType = profiledArrayType.SetHasNoMissingValues(valueType.HasNoMissingValues());
ChangeValueType(this->currentBlock, CurrentBlockData()->FindValue(opnd->AsRegOpnd()->m_sym), valueType, false);
}
else if (!useAggressiveSpecialization && valueType.IsLikelyNativeArray() &&
(profiledArrayType.HasVarElements() || (valueType.HasIntElements() && profiledArrayType.HasFloatElements())))
{
// Merge array type we pulled from profile with type propagated by dataflow.
valueType = valueType.Merge(profiledArrayType).SetHasNoMissingValues(valueType.HasNoMissingValues());
ChangeValueType(this->currentBlock, CurrentBlockData()->FindValue(opnd->AsRegOpnd()->m_sym), valueType, false);
}
}
}

Expand Down Expand Up @@ -13391,11 +13406,9 @@ GlobOpt::OptArraySrc(IR::Instr * *const instrRef)
if (!baseValueInLoopLandingPad->GetValueInfo()->CanMergeToSpecificObjectType())
{
ValueType landingPadValueType = baseValueInLoopLandingPad->GetValueInfo()->Type();
Assert(landingPadValueType.IsSimilar(baseValueType) ||
(
landingPadValueType.IsLikelyNativeArray() &&
landingPadValueType.Merge(baseValueType).IsSimilar(baseValueType)
)
Assert(landingPadValueType.IsSimilar(baseValueType)
|| (landingPadValueType.IsLikelyNativeArray() && landingPadValueType.Merge(baseValueType).IsSimilar(baseValueType))
|| (baseValueType.IsLikelyNativeArray() && baseValueType.Merge(landingPadValueType).IsSimilar(landingPadValueType))
);
}
#endif
Expand Down
6 changes: 4 additions & 2 deletions lib/Backend/IRBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4482,9 +4482,10 @@ IRBuilder::BuildProfiledElementCP(Js::OpCode newOpcode, uint32 offset, Js::RegSl

ValueType arrayType = ValueType::Uninitialized;

const Js::LdLenInfo * ldLenInfo = nullptr;
if (m_func->HasProfileInfo())
{
const Js::LdLenInfo * ldLenInfo = m_func->GetReadOnlyProfileInfo()->GetLdLenInfo(profileId);
ldLenInfo = m_func->GetReadOnlyProfileInfo()->GetLdLenInfo(profileId);
arrayType = (ldLenInfo->GetArrayType());
if (arrayType.IsLikelyNativeArray() &&
(
Expand Down Expand Up @@ -4522,7 +4523,8 @@ IRBuilder::BuildProfiledElementCP(Js::OpCode newOpcode, uint32 offset, Js::RegSl
IR::ProfiledInstr * profiledInstr = IR::ProfiledInstr::New(newOpcode, dstOpnd, fieldSymOpnd, m_func);
instr = profiledInstr;
profiledInstr->u.FldInfo() = *(m_func->GetReadOnlyProfileInfo()->GetFldInfo(inlineCacheIndex));
profiledInstr->u.LdLenInfo().GetArrayType() = arrayType;
profiledInstr->u.LdLenInfo() = *ldLenInfo;
profiledInstr->u.LdLenInfo().arrayType = arrayType;
wasNotProfiled = !profiledInstr->u.FldInfo().WasLdFldProfiled();
dstOpnd->SetValueType(instr->AsProfiledInstr()->u.FldInfo().valueType);
#if ENABLE_DEBUG_CONFIG_OPTIONS
Expand Down
1 change: 1 addition & 0 deletions lib/Backend/JITTimeProfileInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ JITTimeProfileInfo::InitializeJITProfileData(
return;
}

CompileAssert(sizeof(LdLenIDL) == sizeof(Js::LdLenInfo));
CompileAssert(sizeof(LdElemIDL) == sizeof(Js::LdElemInfo));
CompileAssert(sizeof(StElemIDL) == sizeof(Js::StElemInfo));

Expand Down
8 changes: 6 additions & 2 deletions lib/Backend/Lower.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8696,11 +8696,13 @@ void Lowerer::LowerProfiledLdElemI(IR::JitProfilingInstr *const instr)
const Var varIndex,
FunctionBody *const functionBody,
const ProfileId profileId,
bool didArrayAccessHelperCall)
bool didArrayAccessHelperCall,
bool bailedOutOnArraySpecialization)
*/

Func *const func = instr->m_func;

m_lowererMD.LoadHelperArgument(instr, IR::IntConstOpnd::New(false, TyInt8, func));
m_lowererMD.LoadHelperArgument(instr, IR::IntConstOpnd::New(false, TyInt8, func));
m_lowererMD.LoadHelperArgument(instr, IR::Opnd::CreateProfileIdOpnd(instr->profileId, func));
m_lowererMD.LoadHelperArgument(instr, CreateFunctionBodyOpnd(func));
Expand Down Expand Up @@ -8731,7 +8733,8 @@ void Lowerer::LowerProfiledStElemI(IR::JitProfilingInstr *const instr, const Js:
FunctionBody *const functionBody,
const ProfileId profileId,
const PropertyOperationFlags flags,
bool didArrayAccessHelperCall)
bool didArrayAccessHelperCall,
bool bailedOutOnArraySpecialization)
*/

Func *const func = instr->m_func;
Expand All @@ -8745,6 +8748,7 @@ void Lowerer::LowerProfiledStElemI(IR::JitProfilingInstr *const instr, const Js:
{
helper = IR::HelperProfiledStElem;
m_lowererMD.LoadHelperArgument(instr, IR::IntConstOpnd::New(false, TyInt8, func));
m_lowererMD.LoadHelperArgument(instr, IR::IntConstOpnd::New(false, TyInt8, func));
m_lowererMD.LoadHelperArgument(instr, IR::IntConstOpnd::New(flags, TyInt32, func, true));
}
m_lowererMD.LoadHelperArgument(instr, IR::Opnd::CreateProfileIdOpnd(instr->profileId, func));
Expand Down
2 changes: 2 additions & 0 deletions lib/JITIDL/JITTypes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ typedef struct ArrayCallSiteIDL
typedef struct LdLenIDL
{
unsigned short arrayType;
byte bits;
IDL_PAD1(0)
} LdLenIDL;

typedef struct LdElemIDL
Expand Down
32 changes: 31 additions & 1 deletion lib/Runtime/Language/DynamicProfileInfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,23 @@ namespace Js

struct LdLenInfo
{
typedef struct { ValueType::TSize f1; } TSize;
typedef struct { ValueType::TSize f1; byte f2; } TSize;

ValueType arrayType;

union
{
struct
{
bool disableAggressiveSpecialization : 1;
};
byte bits;
};

LdLenInfo() : bits(0)
{
}

void Merge(const LdLenInfo & other)
{
arrayType = arrayType.Merge(other.arrayType);
Expand All @@ -192,6 +205,11 @@ namespace Js
{
return arrayType;
}

bool IsAggressiveSpecializationDisabled() const
{
return disableAggressiveSpecialization;
}
};
CompileAssert(sizeof(LdLenInfo::TSize) == sizeof(LdLenInfo));

Expand All @@ -206,6 +224,7 @@ namespace Js
{
bool wasProfiled : 1;
bool neededHelperCall : 1;
bool disableAggressiveSpecialization : 1;
};
byte bits;
};
Expand Down Expand Up @@ -241,6 +260,11 @@ namespace Js
{
return neededHelperCall;
}

bool IsAggressiveSpecializationDisabled() const
{
return disableAggressiveSpecialization;
}
};

struct StElemInfo
Expand All @@ -257,6 +281,7 @@ namespace Js
bool neededHelperCall : 1;
bool storedOutsideHeadSegmentBounds : 1;
bool storedOutsideArrayBounds : 1;
bool disableAggressiveSpecialization : 1;
};
byte bits;
};
Expand Down Expand Up @@ -306,6 +331,11 @@ namespace Js
{
return storedOutsideArrayBounds;
}

bool IsAggressiveSpecializationDisabled() const
{
return disableAggressiveSpecialization;
}
};

struct ArrayCallSiteInfo
Expand Down
16 changes: 12 additions & 4 deletions lib/Runtime/Language/InterpreterStackFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4977,9 +4977,10 @@ namespace Js
GetReg(playout->Element),
m_functionBody,
playout->profileId,
this->TestFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall)));
this->TestFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall),
this->TestFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArraySpecialization)));

this->ClearFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall);
this->ClearFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall | InterpreterStackFrameFlags_ProcessingBailOutOnArraySpecialization);

threadContext->CheckAndResetImplicitCallAccessorFlag();
threadContext->AddImplicitCallFlags(savedImplicitCallFlags);
Expand Down Expand Up @@ -5077,9 +5078,10 @@ namespace Js
m_functionBody,
playout->profileId,
flags,
this->TestFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall));
this->TestFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall),
this->TestFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArraySpecialization));

this->ClearFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall);
this->ClearFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall | InterpreterStackFrameFlags_ProcessingBailOutOnArraySpecialization);

threadContext->CheckAndResetImplicitCallAccessorFlag();
threadContext->AddImplicitCallFlags(savedImplicitCallFlags);
Expand Down Expand Up @@ -8412,6 +8414,12 @@ const byte * InterpreterStackFrame::OP_ProfiledLoopBodyStart(uint loopId)
ldLenInfo.arrayType = ValueType::Uninitialized.Merge(instance);
profileData->RecordLengthLoad(functionBody, playout->profileId, ldLenInfo);

if (this->TestFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArraySpecialization))
{
ldLenInfo.disableAggressiveSpecialization = true;
this->ClearFlags(InterpreterStackFrameFlags_ProcessingBailOutOnArraySpecialization);
}

ThreadContext* threadContext = this->GetScriptContext()->GetThreadContext();
ImplicitCallFlags savedImplicitCallFlags = threadContext->GetImplicitCallFlags();
threadContext->ClearImplicitCallFlags();
Expand Down
1 change: 1 addition & 0 deletions lib/Runtime/Language/InterpreterStackFrame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ namespace Js
InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall = 0x10,
InterpreterStackFrameFlags_ProcessingBailOutFromEHCode = 0x20,
InterpreterStackFrameFlags_FromBailOutInInlinee = 0x40,
InterpreterStackFrameFlags_ProcessingBailOutOnArraySpecialization = 0x80,
InterpreterStackFrameFlags_All = 0xFFFF,
};

Expand Down
18 changes: 15 additions & 3 deletions lib/Runtime/Language/ProfilingHelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ namespace Js
const Var varIndex,
FunctionBody *const functionBody,
const ProfileId profileId,
bool didArrayAccessHelperCall)
bool didArrayAccessHelperCall,
bool bailedOutOnArraySpecialization)
{
Assert(base);
Assert(varIndex);
Expand All @@ -21,6 +22,11 @@ namespace Js

LdElemInfo ldElemInfo;

if (bailedOutOnArraySpecialization)
{
ldElemInfo.disableAggressiveSpecialization = true;
}

// Only enable fast path if the javascript array is not cross site
#if ENABLE_COPYONACCESS_ARRAY
JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(base);
Expand Down Expand Up @@ -197,7 +203,7 @@ namespace Js
FunctionBody *const functionBody,
const ProfileId profileId)
{
ProfiledStElem(base, varIndex, value, functionBody, profileId, PropertyOperation_None, false);
ProfiledStElem(base, varIndex, value, functionBody, profileId, PropertyOperation_None, false, false);
}

void ProfilingHelpers::ProfiledStElem(
Expand All @@ -207,7 +213,8 @@ namespace Js
FunctionBody *const functionBody,
const ProfileId profileId,
const PropertyOperationFlags flags,
bool didArrayAccessHelperCall)
bool didArrayAccessHelperCall,
bool bailedOutOnArraySpecialization)
{
Assert(base);
Assert(varIndex);
Expand All @@ -217,6 +224,11 @@ namespace Js

StElemInfo stElemInfo;

if (bailedOutOnArraySpecialization)
{
stElemInfo.disableAggressiveSpecialization = true;
}

// Only enable fast path if the javascript array is not cross site
const bool isJsArray = !TaggedNumber::Is(base) && VirtualTableInfo<JavascriptArray>::HasVirtualTable(base);
ScriptContext *const scriptContext = functionBody->GetScriptContext();
Expand Down
4 changes: 2 additions & 2 deletions lib/Runtime/Language/ProfilingHelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ namespace Js
class ProfilingHelpers
{
public:
static Var ProfiledLdElem(const Var base, const Var varIndex, FunctionBody *const functionBody, const ProfileId profileId, bool didArrayAccessHelperCall);
static Var ProfiledLdElem(const Var base, const Var varIndex, FunctionBody *const functionBody, const ProfileId profileId, bool didArrayAccessHelperCall, bool bailedOutOnArraySpecialization);
static Var ProfiledLdElem_FastPath(JavascriptArray *const array, const Var varIndex, ScriptContext *const scriptContext, LdElemInfo *const ldElemInfo = nullptr);

public:
static void ProfiledStElem_DefaultFlags(const Var base, const Var varIndex, const Var value, FunctionBody *const functionBody, const ProfileId profileId);
static void ProfiledStElem(const Var base, const Var varIndex, const Var value, FunctionBody *const functionBody, const ProfileId profileId, const PropertyOperationFlags flags, bool didArrayAccessHelperCall);
static void ProfiledStElem(const Var base, const Var varIndex, const Var value, FunctionBody *const functionBody, const ProfileId profileId, const PropertyOperationFlags flags, bool didArrayAccessHelperCall, bool bailedOutOnArraySpecialization);
static void ProfiledStElem_FastPath(JavascriptArray *const array, const Var varIndex, const Var value, ScriptContext *const scriptContext, const PropertyOperationFlags flags, StElemInfo *const stElemInfo = nullptr);

public:
Expand Down

0 comments on commit 5ed292c

Please sign in to comment.