Skip to content
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.

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

17-07 ChakraCore servicing release #3341

Merged
merged 12 commits into from
Jul 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 1 addition & 4 deletions lib/Backend/BackwardPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6854,10 +6854,7 @@ BackwardPass::TrackNoImplicitCallInlinees(IR::Instr *instr)
|| OpCodeAttr::CallInstr(instr->m_opcode)
|| instr->CallsAccessor()
|| GlobOpt::MayNeedBailOnImplicitCall(instr, nullptr, nullptr)
|| instr->m_opcode == Js::OpCode::LdHeapArguments
|| instr->m_opcode == Js::OpCode::LdLetHeapArguments
|| instr->m_opcode == Js::OpCode::LdHeapArgsCached
|| instr->m_opcode == Js::OpCode::LdLetHeapArgsCached
|| instr->HasAnyLoadHeapArgsOpCode()
|| instr->m_opcode == Js::OpCode::LdFuncExpr)
{
// This func has instrs with bailouts or implicit calls
Expand Down
2 changes: 1 addition & 1 deletion lib/Backend/Func.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
hasThrow(false),
hasNonSimpleParams(false),
hasUnoptimizedArgumentsAcccess(false),
hasApplyTargetInlining(false),
applyTargetInliningRemovedArgumentsAccess(false),
hasImplicitCalls(false),
hasTempObjectProducingInstr(false),
isInlinedConstructor(isInlinedConstructor),
Expand Down
24 changes: 21 additions & 3 deletions lib/Backend/Func.h
Original file line number Diff line number Diff line change
Expand Up @@ -509,12 +509,24 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
IR::SymOpnd *GetNextInlineeFrameArgCountSlotOpnd()
{
Assert(!this->m_hasInlineArgsOpt);
if (this->m_hasInlineArgsOpt)
{
// If the function has inlineArgsOpt turned on, jitted code will not write to stack slots for inlinee's function object
// and arguments, until needed. If we attempt to read from those slots, we may be reading uninitialized memory.
throw Js::OperationAbortedException();
}
return GetInlineeOpndAtOffset((Js::Constants::InlineeMetaArgCount + actualCount) * MachPtr);
}

IR::SymOpnd *GetInlineeFunctionObjectSlotOpnd()
{
Assert(!this->m_hasInlineArgsOpt);
if (this->m_hasInlineArgsOpt)
{
// If the function has inlineArgsOpt turned on, jitted code will not write to stack slots for inlinee's function object
// and arguments, until needed. If we attempt to read from those slots, we may be reading uninitialized memory.
throw Js::OperationAbortedException();
}
return GetInlineeOpndAtOffset(Js::Constants::InlineeMetaArgIndex_FunctionObject * MachPtr);
}

Expand All @@ -526,6 +538,12 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
IR::SymOpnd *GetInlineeArgvSlotOpnd()
{
Assert(!this->m_hasInlineArgsOpt);
if (this->m_hasInlineArgsOpt)
{
// If the function has inlineArgsOpt turned on, jitted code will not write to stack slots for inlinee's function object
// and arguments, until needed. If we attempt to read from those slots, we may be reading uninitialized memory.
throw Js::OperationAbortedException();
}
return GetInlineeOpndAtOffset(Js::Constants::InlineeMetaArgIndex_Argv * MachPtr);
}

Expand Down Expand Up @@ -686,7 +704,7 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
bool hasThrow : 1;
bool hasUnoptimizedArgumentsAcccess : 1; // True if there are any arguments access beyond the simple case of this.apply pattern
bool m_canDoInlineArgsOpt : 1;
bool hasApplyTargetInlining:1;
bool applyTargetInliningRemovedArgumentsAccess :1;
bool isGetterSetter : 1;
const bool isInlinedConstructor: 1;
bool hasImplicitCalls: 1;
Expand Down Expand Up @@ -804,8 +822,8 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
}
}

bool GetHasApplyTargetInlining() const { return this->hasApplyTargetInlining;}
void SetHasApplyTargetInlining() { this->hasApplyTargetInlining = true;}
bool GetApplyTargetInliningRemovedArgumentsAccess() const { return this->applyTargetInliningRemovedArgumentsAccess;}
void SetApplyTargetInliningRemovedArgumentsAccess() { this->applyTargetInliningRemovedArgumentsAccess = true;}

bool GetHasMarkTempObjects() const { return this->hasMarkTempObjects; }
void SetHasMarkTempObjects() { this->hasMarkTempObjects = true; }
Expand Down
12 changes: 12 additions & 0 deletions lib/Backend/GlobOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6220,6 +6220,18 @@ GlobOpt::FindObjectTypeValue(SymID typeSymId, GlobHashTable *valueNumberMap, BVS
{
return nullptr;
}
return FindObjectTypeValueNoLivenessCheck(typeSymId, valueNumberMap);
}

Value *
GlobOpt::FindObjectTypeValueNoLivenessCheck(StackSym* typeSym, BasicBlock* block)
{
return FindObjectTypeValueNoLivenessCheck(typeSym->m_id, block->globOptData.symToValueMap);
}

Value *
GlobOpt::FindObjectTypeValueNoLivenessCheck(SymID typeSymId, GlobHashTable *valueNumberMap)
{
Value* value = FindValueFromHashTable(valueNumberMap, typeSymId);
Assert(value == nullptr || value->GetValueInfo()->IsJsType());
return value;
Expand Down
3 changes: 3 additions & 0 deletions lib/Backend/GlobOpt.h
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,8 @@ class GlobOpt
Value* FindObjectTypeValue(SymID typeSymId, BasicBlock* block);
Value * FindObjectTypeValue(StackSym* typeSym, GlobHashTable *valueNumberMap, BVSparse<JitArenaAllocator>* liveFields);
Value * FindObjectTypeValue(SymID typeSymId, GlobHashTable *valueNumberMap, BVSparse<JitArenaAllocator>* liveFields);
Value * FindObjectTypeValueNoLivenessCheck(StackSym* typeSym, BasicBlock* block);
Value * FindObjectTypeValueNoLivenessCheck(SymID typeSymId, GlobHashTable *valueNumberMap);
Value * FindFuturePropertyValue(PropertySym *const propertySym);
IR::Opnd * CopyProp(IR::Opnd *opnd, IR::Instr *instr, Value *val, IR::IndirOpnd *parentIndirOpnd = nullptr);
IR::Opnd * CopyPropReplaceOpnd(IR::Instr * instr, IR::Opnd * opnd, StackSym * copySym, IR::IndirOpnd *parentIndirOpnd = nullptr);
Expand Down Expand Up @@ -1760,6 +1762,7 @@ class GlobOpt
bool PreparePropertySymOpndForTypeCheckSeq(IR::PropertySymOpnd *propertySymOpnd, IR::Instr * instr, Loop *loop);
static bool AreTypeSetsIdentical(Js::EquivalentTypeSet * leftTypeSet, Js::EquivalentTypeSet * rightTypeSet);
static bool IsSubsetOf(Js::EquivalentTypeSet * leftTypeSet, Js::EquivalentTypeSet * rightTypeSet);
static bool CompareCurrentTypesWithExpectedTypes(JsTypeValueInfo *valueInfo, IR::PropertySymOpnd * propertySymOpnd);
bool ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd);
bool CheckIfInstrInTypeCheckSeqEmitsTypeCheck(IR::Instr* instr, IR::PropertySymOpnd *opnd);
template<bool makeChanges>
Expand Down
88 changes: 88 additions & 0 deletions lib/Backend/GlobOptFields.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2310,6 +2310,79 @@ GlobOpt::IsSubsetOf(Js::EquivalentTypeSet * leftTypeSet, Js::EquivalentTypeSet *
return Js::EquivalentTypeSet::IsSubsetOf(leftTypeSet, rightTypeSet);
}

bool
GlobOpt::CompareCurrentTypesWithExpectedTypes(JsTypeValueInfo *valueInfo, IR::PropertySymOpnd * propertySymOpnd)
{
bool isTypeDead = propertySymOpnd->IsTypeDead();

if (valueInfo == nullptr || (valueInfo->GetJsType() == nullptr && valueInfo->GetJsTypeSet() == nullptr))
{
// No upstream types. Do a type check.
return !isTypeDead;
}

if (!propertySymOpnd->HasEquivalentTypeSet() || propertySymOpnd->NeedsMonoCheck())
{
JITTypeHolder opndType = propertySymOpnd->GetType();

if (valueInfo->GetJsType() != nullptr)
{
if (valueInfo->GetJsType() == propertySymOpnd->GetType())
{
return true;
}
if (propertySymOpnd->HasInitialType() && valueInfo->GetJsType() == propertySymOpnd->GetInitialType())
{
return !isTypeDead;
}
return false;
}
else
{
Assert(valueInfo->GetJsTypeSet());
Js::EquivalentTypeSet *valueTypeSet = valueInfo->GetJsTypeSet();

if (valueTypeSet->Contains(opndType))
{
return !isTypeDead;
}
if (propertySymOpnd->HasInitialType() && valueTypeSet->Contains(propertySymOpnd->GetInitialType()))
{
return !isTypeDead;
}
return false;
}
}
else
{
Js::EquivalentTypeSet * opndTypeSet = propertySymOpnd->GetEquivalentTypeSet();

if (valueInfo->GetJsType() != nullptr)
{
uint16 checkedTypeSetIndex;
if (opndTypeSet->Contains(valueInfo->GetJsType(), &checkedTypeSetIndex))
{
return true;
}
return false;
}
else
{
if (IsSubsetOf(valueInfo->GetJsTypeSet(), opndTypeSet))
{
return true;
}
if (propertySymOpnd->IsMono() ?
valueInfo->GetJsTypeSet()->Contains(propertySymOpnd->GetFirstEquivalentType()) :
IsSubsetOf(opndTypeSet, valueInfo->GetJsTypeSet()))
{
return true;
}
return false;
}
}
}

bool
GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd)
{
Expand Down Expand Up @@ -3067,6 +3140,21 @@ GlobOpt::CopyPropPropertySymObj(IR::SymOpnd *symOpnd, IR::Instr *instr)
{
IR::PropertySymOpnd *propertySymOpnd = symOpnd->AsPropertySymOpnd();

if (propertySymOpnd->IsTypeCheckSeqCandidate())
{
// If the new pointer sym's expected type(s) don't match those in the inline-cache-based data for this access,
// we probably have a mismatch and can't safely objtypespec. If the saved objtypespecfldinfo isn't right for
// the new type, then we'll do an incorrect property access.
StackSym * newTypeSym = copySym->GetObjectTypeSym();
Value * newValue = this->FindObjectTypeValueNoLivenessCheck(newTypeSym, this->currentBlock);
JsTypeValueInfo * newValueInfo = newValue ? newValue->GetValueInfo()->AsJsType() : nullptr;
bool shouldOptimize = CompareCurrentTypesWithExpectedTypes(newValueInfo, propertySymOpnd);
if (!shouldOptimize)
{
propertySymOpnd->SetTypeCheckSeqCandidate(false);
}
}

// This is no longer strictly necessary, since we don't set the type dead bits in the initial
// backward pass, but let's keep it around for now in case we choose to revert to the old model.
propertySymOpnd->SetTypeDeadIfTypeCheckSeqCandidate(false);
Expand Down
49 changes: 37 additions & 12 deletions lib/Backend/Inline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2701,6 +2701,13 @@ bool Inline::InlineApplyScriptTarget(IR::Instr *callInstr, const FunctionJITTime
return false;
});

// If the arguments object was passed in as the first argument to apply,
// 'arguments' access continues to exist even after apply target inlining
if (!HasArgumentsAccess(explicitThisArgOut))
{
callInstr->m_func->SetApplyTargetInliningRemovedArgumentsAccess();
}

if (safeThis)
{
IR::Instr * byteCodeArgOutCapture = explicitThisArgOut->GetBytecodeArgOutCapture();
Expand Down Expand Up @@ -3134,11 +3141,6 @@ Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const Functi
return false;
}

if (isApplyTarget)
{
callInstr->m_func->SetHasApplyTargetInlining();
}

Assert(callInstr->m_opcode == originalCallOpCode);
callInstr->ReplaceSrc1(builtInLdInstr->GetDst());

Expand Down Expand Up @@ -5281,21 +5283,39 @@ Inline::IsArgumentsOpnd(IR::Opnd* opnd, SymID argumentsSymId)
return false;
}

bool
Inline::IsArgumentsOpnd(IR::Opnd* opnd)
{
IR::Opnd * checkOpnd = opnd;
while (checkOpnd)
{
if (checkOpnd->IsArgumentsObject())
{
return true;
}
checkOpnd = checkOpnd->GetStackSym() && checkOpnd->GetStackSym()->IsSingleDef() ? checkOpnd->GetStackSym()->GetInstrDef()->GetSrc1() : nullptr;
}

return false;
}

bool
Inline::HasArgumentsAccess(IR::Opnd *opnd, SymID argumentsSymId)
{
// We should look at dst last to correctly handle cases where it's the same as one of the src operands.
if (opnd)
IR::Opnd * checkOpnd = opnd;
while (checkOpnd)
{
if (opnd->IsRegOpnd() || opnd->IsSymOpnd() || opnd->IsIndirOpnd())
if (checkOpnd->IsRegOpnd() || checkOpnd->IsSymOpnd() || checkOpnd->IsIndirOpnd())
{
if (IsArgumentsOpnd(opnd, argumentsSymId))
if (IsArgumentsOpnd(checkOpnd, argumentsSymId))
{
return true;
}
}
checkOpnd = checkOpnd->GetStackSym() && checkOpnd->GetStackSym()->IsSingleDef() ? checkOpnd->GetStackSym()->GetInstrDef()->GetSrc1() : nullptr;
}

return false;
}

Expand Down Expand Up @@ -5328,6 +5348,13 @@ Inline::HasArgumentsAccess(IR::Instr * instr, SymID argumentsSymId)
return false;
}

bool
Inline::HasArgumentsAccess(IR::Instr * instr)
{
return (instr->GetSrc1() && IsArgumentsOpnd(instr->GetSrc1())) ||
(instr->GetSrc2() && IsArgumentsOpnd(instr->GetSrc2()));
}

bool
Inline::GetInlineeHasArgumentObject(Func * inlinee)
{
Expand All @@ -5339,14 +5366,12 @@ Inline::GetInlineeHasArgumentObject(Func * inlinee)

// Inlinee has arguments access

if (!inlinee->GetHasApplyTargetInlining())
if (!inlinee->GetApplyTargetInliningRemovedArgumentsAccess())
{
// There is no apply target inlining (this.init.apply(this, arguments))
// So arguments access continues to exist
return true;
}

// Its possible there is no more arguments access after we inline apply target validate the same.
// Its possible there is no more arguments access after we inline apply target; validate the same.
// This sounds expensive, but we are only walking inlinee which has apply target inlining optimization enabled.
// Also we walk only instruction in that inlinee and not nested inlinees. So it is not expensive.
SymID argumentsSymId = 0;
Expand Down
2 changes: 2 additions & 0 deletions lib/Backend/Inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ class Inline
IR::Instr * RemoveLdThis(IR::Instr *instr);
bool GetInlineeHasArgumentObject(Func * inlinee);
bool HasArgumentsAccess(IR::Instr * instr, SymID argumentsSymId);
bool HasArgumentsAccess(IR::Instr * instr);
bool HasArgumentsAccess(IR::Opnd * opnd, SymID argumentsSymId);
bool IsArgumentsOpnd(IR::Opnd* opnd,SymID argumentsSymId);
bool IsArgumentsOpnd(IR::Opnd* opnd);
void Cleanup(IR::Instr *callInstr);
IR::PropertySymOpnd* GetMethodLdOpndForCallInstr(IR::Instr* callInstr);
#ifdef ENABLE_SIMDJS
Expand Down
Loading