Skip to content

Commit

Permalink
[MERGE #5663 @pleath] Remove redundant aux slot ptr loads
Browse files Browse the repository at this point in the history 
Merge pull request #5663 from pleath:auxslotopt

Modify and re-enable this optimization. Track availability of previously loaded aux slot pointers independently of type symbols. Also track upward-exposed uses so that we can avoid unnecessary reloads. (Note that the optimization relies on the change to disable dead-storing of type checks on property stores.)
  • Loading branch information
@pleath
pleath committed Sep 13, 2018
2 parents 50eaeec + 8c5332b commit 9976ab2
Show file tree
Hide file tree
Showing 10 changed files with 214 additions and 89 deletions.
80 changes: 45 additions & 35 deletions lib/Backend/BackwardPass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ BackwardPass::MergeSuccBlocksInfo(BasicBlock * block)
this->func->GetDebugNumberSet(debugStringBuffer),
block->GetBlockNum(), blockSucc->GetBlockNum());

auto fixupFrom = [block, blockSucc, this](Bucket<AddPropertyCacheBucket> &bucket)
auto fixupFrom = [block, blockSucc, upwardExposedUses, this](Bucket<AddPropertyCacheBucket> &bucket)
{
AddPropertyCacheBucket *fromData = &bucket.element;
if (fromData->GetInitialType() == nullptr ||
Expand All @@ -729,10 +729,10 @@ BackwardPass::MergeSuccBlocksInfo(BasicBlock * block)
return;
}

this->InsertTypeTransitionsAtPriorSuccessors(block, blockSucc, bucket.value, fromData);
this->InsertTypeTransitionsAtPriorSuccessors(block, blockSucc, bucket.value, fromData, upwardExposedUses);
};

auto fixupTo = [blockSucc, this](Bucket<AddPropertyCacheBucket> &bucket)
auto fixupTo = [blockSucc, upwardExposedUses, this](Bucket<AddPropertyCacheBucket> &bucket)
{
AddPropertyCacheBucket *toData = &bucket.element;
if (toData->GetInitialType() == nullptr ||
Expand All @@ -741,7 +741,7 @@ BackwardPass::MergeSuccBlocksInfo(BasicBlock * block)
return;
}

this->InsertTypeTransitionAtBlock(blockSucc, bucket.value, toData);
this->InsertTypeTransitionAtBlock(blockSucc, bucket.value, toData, upwardExposedUses);
};

if (blockSucc->stackSymToFinalType != nullptr)
Expand Down Expand Up @@ -4614,7 +4614,7 @@ BackwardPass::ProcessNewScObject(IR::Instr* instr)
Assert(pBucket->GetInitialType() == ctorCache->GetType());
if (!this->IsPrePass())
{
this->InsertTypeTransition(instr->m_next, objSym, pBucket);
this->InsertTypeTransition(instr->m_next, objSym, pBucket, block->upwardExposedUses);
}
#if DBG
pBucket->deadStoreUnavailableInitialType = pBucket->GetInitialType();
Expand Down Expand Up @@ -5192,7 +5192,7 @@ BackwardPass::ProcessPropertySymOpndUse(IR::PropertySymOpnd * opnd)
pBucket->GetFinalType() != nullptr &&
pBucket->GetFinalType() != pBucket->GetInitialType())
{
this->InsertTypeTransition(this->currentInstr->m_next, baseSym, pBucket);
this->InsertTypeTransition(this->currentInstr->m_next, baseSym, pBucket, block->upwardExposedUses);
pBucket->SetFinalType(pBucket->GetInitialType());
}
}
Expand All @@ -5211,9 +5211,6 @@ BackwardPass::ProcessPropertySymOpndUse(IR::PropertySymOpnd * opnd)
void
BackwardPass::TrackObjTypeSpecProperties(IR::PropertySymOpnd *opnd, BasicBlock *block)
{
StackSym *auxSlotPtrSym = nullptr;
bool auxSlotPtrUpwardExposed = false;

Assert(tag == Js::DeadStorePhase);
Assert(opnd->IsTypeCheckSeqCandidate());

Expand Down Expand Up @@ -5280,7 +5277,6 @@ BackwardPass::TrackObjTypeSpecProperties(IR::PropertySymOpnd *opnd, BasicBlock *
#endif

bucket->AddToGuardedPropertyOps(opnd->GetObjTypeSpecFldId());
auxSlotPtrUpwardExposed = PHASE_ON(Js::ReuseAuxSlotPtrPhase, this->func) && opnd->UsesAuxSlot() && !opnd->IsLoadedFromProto() && opnd->IsTypeChecked();

if (opnd->NeedsMonoCheck())
{
Expand Down Expand Up @@ -5327,12 +5323,6 @@ BackwardPass::TrackObjTypeSpecProperties(IR::PropertySymOpnd *opnd, BasicBlock *
bucket->SetGuardedPropertyOps(nullptr);
JitAdelete(this->tempAlloc, guardedPropertyOps);
block->stackSymToGuardedProperties->Clear(objSym->m_id);
auxSlotPtrSym = opnd->GetAuxSlotPtrSym();
if (auxSlotPtrSym)
{
this->currentBlock->upwardExposedUses->Clear(auxSlotPtrSym->m_id);
}
auxSlotPtrUpwardExposed = false;
}
}
#if DBG
Expand All @@ -5351,11 +5341,25 @@ BackwardPass::TrackObjTypeSpecProperties(IR::PropertySymOpnd *opnd, BasicBlock *
opnd->SetGuardedPropOp(opnd->GetObjTypeSpecFldId());
}

if (auxSlotPtrUpwardExposed)
if (opnd->UsesAuxSlot() && opnd->IsTypeCheckSeqParticipant() && !opnd->HasTypeMismatch() && !opnd->IsLoadedFromProto())
{
// This is an upward-exposed use of the aux slot pointer.
auxSlotPtrSym = opnd->EnsureAuxSlotPtrSym(this->func);
this->currentBlock->upwardExposedUses->Set(auxSlotPtrSym->m_id);
bool auxSlotPtrUpwardExposed = false;
StackSym *auxSlotPtrSym = opnd->GetAuxSlotPtrSym();
if (opnd->IsAuxSlotPtrSymAvailable())
{
// This is an upward-exposed use of the aux slot pointer.
Assert(auxSlotPtrSym);
auxSlotPtrUpwardExposed = this->currentBlock->upwardExposedUses->TestAndSet(auxSlotPtrSym->m_id);
}
else if (auxSlotPtrSym != nullptr)
{
// The aux slot pointer is not upward-exposed at this point.
auxSlotPtrUpwardExposed = this->currentBlock->upwardExposedUses->TestAndClear(auxSlotPtrSym->m_id);
}
if (!this->IsPrePass() && auxSlotPtrUpwardExposed)
{
opnd->SetProducesAuxSlotPtr(true);
}
}
}

Expand Down Expand Up @@ -5611,16 +5615,18 @@ BackwardPass::TrackAddPropertyTypes(IR::PropertySymOpnd *opnd, BasicBlock *block
}

void
BackwardPass::InsertTypeTransition(IR::Instr *instrInsertBefore, int symId, AddPropertyCacheBucket *data)
BackwardPass::InsertTypeTransition(IR::Instr *instrInsertBefore, int symId, AddPropertyCacheBucket *data, BVSparse<JitArenaAllocator>* upwardExposedUses)
{
StackSym *objSym = this->func->m_symTable->FindStackSym(symId);
Assert(objSym);
this->InsertTypeTransition(instrInsertBefore, objSym, data);
this->InsertTypeTransition(instrInsertBefore, objSym, data, upwardExposedUses);
}

void
BackwardPass::InsertTypeTransition(IR::Instr *instrInsertBefore, StackSym *objSym, AddPropertyCacheBucket *data)
BackwardPass::InsertTypeTransition(IR::Instr *instrInsertBefore, StackSym *objSym, AddPropertyCacheBucket *data, BVSparse<JitArenaAllocator>* upwardExposedUses)
{
Assert(!this->IsPrePass());

IR::RegOpnd *baseOpnd = IR::RegOpnd::New(objSym, TyMachReg, this->func);
baseOpnd->SetIsJITOptimizedReg(true);

Expand All @@ -5637,7 +5643,7 @@ BackwardPass::InsertTypeTransition(IR::Instr *instrInsertBefore, StackSym *objSy
IR::Instr *adjustTypeInstr =
IR::Instr::New(Js::OpCode::AdjustObjType, finalTypeOpnd, baseOpnd, initialTypeOpnd, this->func);

if (this->currentBlock->upwardExposedUses)
if (upwardExposedUses)
{
// If this type change causes a slot adjustment, the aux slot pointer (if any) will be reloaded here, so take it out of upwardExposedUses.
int oldCount;
Expand All @@ -5651,7 +5657,10 @@ BackwardPass::InsertTypeTransition(IR::Instr *instrInsertBefore, StackSym *objSy
StackSym *auxSlotPtrSym = baseOpnd->m_sym->GetAuxSlotPtrSym();
if (auxSlotPtrSym)
{
this->currentBlock->upwardExposedUses->Clear(auxSlotPtrSym->m_id);
if (upwardExposedUses->Test(auxSlotPtrSym->m_id))
{
adjustTypeInstr->m_opcode = Js::OpCode::AdjustObjTypeReloadAuxSlotPtr;
}
}
}
}
Expand All @@ -5660,7 +5669,7 @@ BackwardPass::InsertTypeTransition(IR::Instr *instrInsertBefore, StackSym *objSy
}

void
BackwardPass::InsertTypeTransitionAfterInstr(IR::Instr *instr, int symId, AddPropertyCacheBucket *data)
BackwardPass::InsertTypeTransitionAfterInstr(IR::Instr *instr, int symId, AddPropertyCacheBucket *data, BVSparse<JitArenaAllocator>* upwardExposedUses)
{
if (!this->IsPrePass())
{
Expand All @@ -5669,11 +5678,11 @@ BackwardPass::InsertTypeTransitionAfterInstr(IR::Instr *instr, int symId, AddPro
{
// The instr with the bailout is something like a branch that may not fall through.
// Insert the transitions instead at the beginning of each successor block.
this->InsertTypeTransitionsAtPriorSuccessors(this->currentBlock, nullptr, symId, data);
this->InsertTypeTransitionsAtPriorSuccessors(this->currentBlock, nullptr, symId, data, upwardExposedUses);
}
else
{
this->InsertTypeTransition(instr->m_next, symId, data);
this->InsertTypeTransition(instr->m_next, symId, data, upwardExposedUses);
}
}
// Note: we could probably clear this entry out of the table, but I don't know
Expand All @@ -5682,7 +5691,7 @@ BackwardPass::InsertTypeTransitionAfterInstr(IR::Instr *instr, int symId, AddPro
}

void
BackwardPass::InsertTypeTransitionAtBlock(BasicBlock *block, int symId, AddPropertyCacheBucket *data)
BackwardPass::InsertTypeTransitionAtBlock(BasicBlock *block, int symId, AddPropertyCacheBucket *data, BVSparse<JitArenaAllocator>* upwardExposedUses)
{
bool inserted = false;
FOREACH_INSTR_IN_BLOCK(instr, block)
Expand All @@ -5705,7 +5714,7 @@ BackwardPass::InsertTypeTransitionAtBlock(BasicBlock *block, int symId, AddPrope
}
else
{
this->InsertTypeTransition(instr, symId, data);
this->InsertTypeTransition(instr, symId, data, upwardExposedUses);
inserted = true;
break;
}
Expand All @@ -5715,7 +5724,7 @@ BackwardPass::InsertTypeTransitionAtBlock(BasicBlock *block, int symId, AddPrope
if (!inserted)
{
Assert(block->GetLastInstr()->m_next);
this->InsertTypeTransition(block->GetLastInstr()->m_next, symId, data);
this->InsertTypeTransition(block->GetLastInstr()->m_next, symId, data, upwardExposedUses);
}
}

Expand All @@ -5724,7 +5733,8 @@ BackwardPass::InsertTypeTransitionsAtPriorSuccessors(
BasicBlock *block,
BasicBlock *blockSucc,
int symId,
AddPropertyCacheBucket *data)
AddPropertyCacheBucket *data,
BVSparse<JitArenaAllocator>* upwardExposedUses)
{
// For each successor of block prior to blockSucc, adjust the type.
FOREACH_SUCCESSOR_BLOCK(blockFix, block)
Expand All @@ -5734,7 +5744,7 @@ BackwardPass::InsertTypeTransitionsAtPriorSuccessors(
return;
}

this->InsertTypeTransitionAtBlock(blockFix, symId, data);
this->InsertTypeTransitionAtBlock(blockFix, symId, data, upwardExposedUses);
}
NEXT_SUCCESSOR_BLOCK;
}
Expand All @@ -5752,7 +5762,7 @@ BackwardPass::InsertTypeTransitionsAtPotentialKills()
// Also do this for ctor cache updates, to avoid putting a type in the ctor cache that extends past
// the end of the ctor that the cache covers.
this->ForEachAddPropertyCacheBucket([&](int symId, AddPropertyCacheBucket *data)->bool {
this->InsertTypeTransitionAfterInstr(instr, symId, data);
this->InsertTypeTransitionAfterInstr(instr, symId, data, this->currentBlock->upwardExposedUses);
return false;
});
}
Expand All @@ -5778,7 +5788,7 @@ BackwardPass::InsertTypeTransitionsAtPotentialKills()
if (this->TransitionUndoesObjectHeaderInlining(data))
{
// We're transitioning from inlined to non-inlined, so we can't push it up any farther.
this->InsertTypeTransitionAfterInstr(instr, symId, data);
this->InsertTypeTransitionAfterInstr(instr, symId, data, this->currentBlock->upwardExposedUses);
}
return false;
});
Expand Down
10 changes: 5 additions & 5 deletions lib/Backend/BackwardPass.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@ class BackwardPass
void TrackObjTypeSpecProperties(IR::PropertySymOpnd *opnd, BasicBlock *block);
void TrackObjTypeSpecWriteGuards(IR::PropertySymOpnd *opnd, BasicBlock *block);
void TrackAddPropertyTypes(IR::PropertySymOpnd *opnd, BasicBlock *block);
void InsertTypeTransition(IR::Instr *instrInsertBefore, int symId, AddPropertyCacheBucket *data);
void InsertTypeTransition(IR::Instr *instrInsertBefore, StackSym *objSym, AddPropertyCacheBucket *data);
void InsertTypeTransitionAtBlock(BasicBlock *block, int symId, AddPropertyCacheBucket *data);
void InsertTypeTransitionsAtPriorSuccessors(BasicBlock *block, BasicBlock *blockSucc, int symId, AddPropertyCacheBucket *data);
void InsertTypeTransitionAfterInstr(IR::Instr *instr, int symId, AddPropertyCacheBucket *data);
void InsertTypeTransition(IR::Instr *instrInsertBefore, int symId, AddPropertyCacheBucket *data, BVSparse<JitArenaAllocator>* upwardExposedUses);
void InsertTypeTransition(IR::Instr *instrInsertBefore, StackSym *objSym, AddPropertyCacheBucket *data, BVSparse<JitArenaAllocator>* upwardExposedUses);
void InsertTypeTransitionAtBlock(BasicBlock *block, int symId, AddPropertyCacheBucket *data, BVSparse<JitArenaAllocator>* upwardExposedUses);
void InsertTypeTransitionsAtPriorSuccessors(BasicBlock *block, BasicBlock *blockSucc, int symId, AddPropertyCacheBucket *data, BVSparse<JitArenaAllocator>* upwardExposedUses);
void InsertTypeTransitionAfterInstr(IR::Instr *instr, int symId, AddPropertyCacheBucket *data, BVSparse<JitArenaAllocator>* upwardExposedUses);
void InsertTypeTransitionsAtPotentialKills();
bool TransitionUndoesObjectHeaderInlining(AddPropertyCacheBucket *data) const;

Expand Down
4 changes: 4 additions & 0 deletions lib/Backend/GlobOpt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ GlobOpt::GlobOpt(Func * func)
updateInductionVariableValueNumber(false),
isPerformingLoopBackEdgeCompensation(false),
currentRegion(nullptr),
auxSlotPtrSyms(nullptr),
changedSymsAfterIncBailoutCandidate(nullptr),
doTypeSpec(
!IsTypeSpecPhaseOff(func)),
Expand Down Expand Up @@ -350,6 +351,8 @@ GlobOpt::ForwardPass()
// changedSymsAfterIncBailoutCandidate helps track building incremental bailout in ForwardPass
this->changedSymsAfterIncBailoutCandidate = JitAnew(alloc, BVSparse<JitArenaAllocator>, alloc);

this->auxSlotPtrSyms = JitAnew(alloc, BVSparse<JitArenaAllocator>, alloc);

#if DBG
this->byteCodeUsesBeforeOpt = JitAnew(this->alloc, BVSparse<JitArenaAllocator>, this->alloc);
if (Js::Configuration::Global.flags.Trace.IsEnabled(Js::FieldCopyPropPhase) && this->DoFunctionFieldCopyProp())
Expand Down Expand Up @@ -431,6 +434,7 @@ GlobOpt::ForwardPass()

// this->alloc will be freed right after return, no need to free it here
this->changedSymsAfterIncBailoutCandidate = nullptr;
this->auxSlotPtrSyms = nullptr;

END_CODEGEN_PHASE(this->func, Js::ForwardPhase);
}
Expand Down
4 changes: 4 additions & 0 deletions lib/Backend/GlobOpt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,8 @@ class GlobOpt

BVSparse<JitArenaAllocator> * changedSymsAfterIncBailoutCandidate;

BVSparse<JitArenaAllocator> * auxSlotPtrSyms;

JitArenaAllocator * alloc;
JitArenaAllocator * tempAlloc;

Expand Down Expand Up @@ -933,6 +935,8 @@ class GlobOpt
bool CheckIfInstrInTypeCheckSeqEmitsTypeCheck(IR::Instr* instr, IR::PropertySymOpnd *opnd);
template<bool makeChanges>
bool ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd, BasicBlock* block, bool updateExistingValue, bool* emitsTypeCheckOut = nullptr, bool* changesTypeValueOut = nullptr, bool *isObjTypeChecked = nullptr);
StackSym * EnsureAuxSlotPtrSym(IR::PropertySymOpnd *opnd);
void KillAuxSlotPtrSyms(IR::PropertySymOpnd *opnd, BasicBlock *block, bool isObjTypeSpecialized);
void KillObjectHeaderInlinedTypeSyms(BasicBlock *block, bool isObjTypeSpecialized, SymID symId = SymID_Invalid);
bool HasLiveObjectHeaderInlinedTypeSym(BasicBlock *block, bool isObjTypeSpecialized, SymID symId = SymID_Invalid);
template<class Fn>
Expand Down
Loading

0 comments on commit 9976ab2

Please sign in to comment.