Skip to content

Commit 2654a02

Browse files
CelluleCellule
committed
Process ByteCodeUsesInstr on the same bytecode offset in DeadStore pass instead of aggregating in Forward's pass
1 parent ba2e065 commit 2654a02

6 files changed

Lines changed: 115 additions & 118 deletions

File tree

‎lib/Backend/BackwardPass.cpp‎

Lines changed: 109 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1996,41 +1996,76 @@ BackwardPass::ProcessBailOutCopyProps(BailOutInfo * bailOutInfo, BVSparse<JitAre
19961996
NEXT_SLISTBASE_ENTRY_EDITING;
19971997
}
19981998

1999-
bool
2000-
BackwardPass::ProcessBailOutInfo(IR::Instr * instr)
1999+
2000+
StackSym*
2001+
BackwardPass::ProcessByteCodeUsesDst(IR::ByteCodeUsesInstr * byteCodeUsesInstr)
20012002
{
2002-
if (this->tag == Js::BackwardPhase)
2003+
Assert(this->DoByteCodeUpwardExposedUsed());
2004+
IR::Opnd * dst = byteCodeUsesInstr->GetDst();
2005+
if (dst)
20032006
{
2004-
// We don't need to fill in the bailout instruction in backward pass
2005-
Assert(this->func->hasBailout || !instr->HasBailOutInfo());
2006-
Assert(!instr->HasBailOutInfo() || instr->GetBailOutInfo()->byteCodeUpwardExposedUsed == nullptr || (this->func->HasTry() && this->func->DoOptimizeTry()));
2007+
IR::RegOpnd * dstRegOpnd = dst->AsRegOpnd();
2008+
StackSym * dstStackSym = dstRegOpnd->m_sym->AsStackSym();
2009+
Assert(!dstRegOpnd->GetIsJITOptimizedReg());
2010+
Assert(dstStackSym->GetByteCodeRegSlot() != Js::Constants::NoRegister);
2011+
if (dstStackSym->GetType() != TyVar)
2012+
{
2013+
dstStackSym = dstStackSym->GetVarEquivSym(nullptr);
2014+
}
20072015

2008-
if (instr->IsByteCodeUsesInstr())
2016+
// If the current region is a Try, symbols in its write-through set shouldn't be cleared.
2017+
// Otherwise, symbols in the write-through set of the first try ancestor shouldn't be cleared.
2018+
if (!this->currentRegion ||
2019+
!this->CheckWriteThroughSymInRegion(this->currentRegion, dstStackSym))
20092020
{
2010-
// FGPeeps inserts bytecodeuses instrs with srcs. We need to look at them to set the proper
2011-
// UpwardExposedUsed info and keep the defs alive.
2012-
// The inliner inserts bytecodeuses instrs withs dsts, but we don't want to look at them for upwardExposedUsed
2013-
// as it would cause real defs to look dead. We use these for bytecodeUpwardExposedUsed info only, which is needed
2014-
// in the dead-store pass only.
2015-
//
2016-
// Handle the source side.
2017-
IR::ByteCodeUsesInstr *byteCodeUsesInstr = instr->AsByteCodeUsesInstr();
2018-
const BVSparse<JitArenaAllocator> * byteCodeUpwardExposedUsed = byteCodeUsesInstr->GetByteCodeUpwardExposedUsed();
2019-
if (byteCodeUpwardExposedUsed != nullptr)
2020-
{
2021-
this->currentBlock->upwardExposedUses->Or(byteCodeUpwardExposedUsed);
2022-
if (this->DoByteCodeUpwardExposedUsed())
2023-
{
2024-
this->currentBlock->byteCodeUpwardExposedUsed->Or(byteCodeUpwardExposedUsed);
2025-
}
2026-
}
2027-
return true;
2021+
this->currentBlock->byteCodeUpwardExposedUsed->Clear(dstStackSym->m_id);
2022+
return dstStackSym;
2023+
20282024
}
2025+
}
2026+
return nullptr;
2027+
}
2028+
2029+
const BVSparse<JitArenaAllocator>*
2030+
BackwardPass::ProcessByteCodeUsesSrcs(IR::ByteCodeUsesInstr * byteCodeUsesInstr)
2031+
{
2032+
Assert(this->DoByteCodeUpwardExposedUsed() || tag == Js::BackwardPhase);
2033+
const BVSparse<JitArenaAllocator>* byteCodeUpwardExposedUsed = byteCodeUsesInstr->GetByteCodeUpwardExposedUsed();
2034+
if (byteCodeUpwardExposedUsed && this->DoByteCodeUpwardExposedUsed())
2035+
{
2036+
this->currentBlock->byteCodeUpwardExposedUsed->Or(byteCodeUpwardExposedUsed);
2037+
}
2038+
return byteCodeUpwardExposedUsed;
2039+
}
2040+
2041+
bool
2042+
BackwardPass::ProcessByteCodeUsesInstr(IR::Instr * instr)
2043+
{
2044+
if (!instr->IsByteCodeUsesInstr())
2045+
{
20292046
return false;
20302047
}
20312048

2032-
if (instr->IsByteCodeUsesInstr())
2049+
IR::ByteCodeUsesInstr * byteCodeUsesInstr = instr->AsByteCodeUsesInstr();
2050+
2051+
if (this->tag == Js::BackwardPhase)
20332052
{
2053+
// FGPeeps inserts bytecodeuses instrs with srcs. We need to look at them to set the proper
2054+
// UpwardExposedUsed info and keep the defs alive.
2055+
// The inliner inserts bytecodeuses instrs withs dsts, but we don't want to look at them for upwardExposedUsed
2056+
// as it would cause real defs to look dead. We use these for bytecodeUpwardExposedUsed info only, which is needed
2057+
// in the dead-store pass only.
2058+
//
2059+
// Handle the source side.
2060+
const BVSparse<JitArenaAllocator>* byteCodeUpwardExposedUsed = ProcessByteCodeUsesSrcs(byteCodeUsesInstr);
2061+
if (byteCodeUpwardExposedUsed != nullptr)
2062+
{
2063+
this->currentBlock->upwardExposedUses->Or(byteCodeUpwardExposedUsed);
2064+
}
2065+
}
2066+
else
2067+
{
2068+
Assert(tag == Js::DeadStorePhase);
20342069
Assert(instr->m_opcode == Js::OpCode::ByteCodeUses);
20352070
#if DBG
20362071
if (this->DoMarkTempObjectVerify() && (this->currentBlock->isDead || !this->func->hasBailout))
@@ -2053,44 +2088,22 @@ BackwardPass::ProcessBailOutInfo(IR::Instr * instr)
20532088

20542089
if (this->func->hasBailout)
20552090
{
2056-
Assert(this->DoByteCodeUpwardExposedUsed());
2057-
20582091
// Just collect the byte code uses, and remove the instruction
20592092
// We are going backward, process the dst first and then the src
2060-
IR::Opnd * dst = instr->GetDst();
2061-
if (dst)
2062-
{
2063-
IR::RegOpnd * dstRegOpnd = dst->AsRegOpnd();
2064-
StackSym * dstStackSym = dstRegOpnd->m_sym->AsStackSym();
2065-
Assert(!dstRegOpnd->GetIsJITOptimizedReg());
2066-
Assert(dstStackSym->GetByteCodeRegSlot() != Js::Constants::NoRegister);
2067-
if (dstStackSym->GetType() != TyVar)
2068-
{
2069-
dstStackSym = dstStackSym->GetVarEquivSym(nullptr);
2070-
}
2071-
2072-
// If the current region is a Try, symbols in its write-through set shouldn't be cleared.
2073-
// Otherwise, symbols in the write-through set of the first try ancestor shouldn't be cleared.
2074-
if (!this->currentRegion ||
2075-
!this->CheckWriteThroughSymInRegion(this->currentRegion, dstStackSym))
2076-
{
2077-
this->currentBlock->byteCodeUpwardExposedUsed->Clear(dstStackSym->m_id);
2093+
StackSym *dstStackSym = ProcessByteCodeUsesDst(byteCodeUsesInstr);
20782094
#if DBG
2079-
// We can only track first level function stack syms right now
2080-
if (dstStackSym->GetByteCodeFunc() == this->func)
2081-
{
2082-
this->currentBlock->byteCodeRestoreSyms[dstStackSym->GetByteCodeRegSlot()] = nullptr;
2083-
}
2084-
#endif
2085-
}
2095+
// We can only track first level function stack syms right now
2096+
if (dstStackSym && dstStackSym->GetByteCodeFunc() == this->func)
2097+
{
2098+
this->currentBlock->byteCodeRestoreSyms[dstStackSym->GetByteCodeRegSlot()] = nullptr;
20862099
}
2100+
#endif
20872101

2088-
IR::ByteCodeUsesInstr *byteCodeUsesInstr = instr->AsByteCodeUsesInstr();
2089-
if (byteCodeUsesInstr->GetByteCodeUpwardExposedUsed() != nullptr)
2090-
{
2091-
this->currentBlock->byteCodeUpwardExposedUsed->Or(byteCodeUsesInstr->GetByteCodeUpwardExposedUsed());
2102+
const BVSparse<JitArenaAllocator>* byteCodeUpwardExposedUsed = ProcessByteCodeUsesSrcs(byteCodeUsesInstr);
20922103
#if DBG
2093-
FOREACH_BITSET_IN_SPARSEBV(symId, byteCodeUsesInstr->GetByteCodeUpwardExposedUsed())
2104+
if (byteCodeUpwardExposedUsed)
2105+
{
2106+
FOREACH_BITSET_IN_SPARSEBV(symId, byteCodeUpwardExposedUsed)
20942107
{
20952108
StackSym * stackSym = this->func->m_symTable->FindStackSym(symId);
20962109
Assert(!stackSym->IsTypeSpec());
@@ -2108,16 +2121,14 @@ BackwardPass::ProcessBailOutInfo(IR::Instr * instr)
21082121
}
21092122
}
21102123
NEXT_BITSET_IN_SPARSEBV;
2111-
#endif
21122124
}
2125+
#endif
21132126

2114-
if(IsCollectionPass())
2127+
if (IsCollectionPass())
21152128
{
21162129
return true;
21172130
}
21182131

2119-
ProcessPendingPreOpBailOutInfo(instr);
2120-
21212132
PropertySym *propertySymUse = byteCodeUsesInstr->propertySymUse;
21222133
if (propertySymUse && !this->currentBlock->isDead)
21232134
{
@@ -2133,13 +2144,27 @@ BackwardPass::ProcessBailOutInfo(IR::Instr * instr)
21332144
}
21342145

21352146
this->currentBlock->RemoveInstr(instr);
2136-
return true;
2147+
}
2148+
return true;
2149+
}
2150+
2151+
bool
2152+
BackwardPass::ProcessBailOutInfo(IR::Instr * instr)
2153+
{
2154+
Assert(!instr->IsByteCodeUsesInstr());
2155+
if (this->tag == Js::BackwardPhase)
2156+
{
2157+
// We don't need to fill in the bailout instruction in backward pass
2158+
Assert(this->func->hasBailout || !instr->HasBailOutInfo());
2159+
Assert(!instr->HasBailOutInfo() || instr->GetBailOutInfo()->byteCodeUpwardExposedUsed == nullptr || (this->func->HasTry() && this->func->DoOptimizeTry()));
2160+
return false;
21372161
}
21382162

21392163
if(IsCollectionPass())
21402164
{
21412165
return false;
21422166
}
2167+
Assert(tag == Js::DeadStorePhase);
21432168

21442169
if (instr->HasBailOutInfo())
21452170
{
@@ -2398,22 +2423,30 @@ BackwardPass::NeedBailOutOnImplicitCallsForTypedArrayStore(IR::Instr* instr)
23982423
return false;
23992424
}
24002425

2401-
void
2426+
IR::Instr*
24022427
BackwardPass::ProcessPendingPreOpBailOutInfo(IR::Instr *const currentInstr)
24032428
{
24042429
Assert(!IsCollectionPass());
24052430

24062431
if(!preOpBailOutInstrToProcess)
24072432
{
2408-
return;
2433+
return currentInstr->m_prev;
24092434
}
2435+
Assert(preOpBailOutInstrToProcess == currentInstr);
24102436

2411-
IR::Instr *const prevInstr = currentInstr->m_prev;
2412-
if(prevInstr &&
2413-
prevInstr->IsByteCodeUsesInstr() &&
2414-
prevInstr->AsByteCodeUsesInstr()->GetByteCodeOffset() == preOpBailOutInstrToProcess->GetByteCodeOffset())
2437+
if (!this->IsPrePass())
24152438
{
2416-
return;
2439+
IR::Instr* prev = preOpBailOutInstrToProcess->m_prev;
2440+
while (prev && preOpBailOutInstrToProcess->CanAggregateByteCodeUsesAcrossInstr(prev))
2441+
{
2442+
IR::Instr* instr = prev;
2443+
prev = prev->m_prev;
2444+
if (instr->IsByteCodeUsesInstrFor(preOpBailOutInstrToProcess))
2445+
{
2446+
// If instr is a ByteCodeUsesInstr, it will remove it
2447+
ProcessByteCodeUsesInstr(instr);
2448+
}
2449+
}
24172450
}
24182451

24192452
// A pre-op bailout instruction was saved for bailout info processing after the instruction and relevant ByteCodeUses
@@ -2423,6 +2456,10 @@ BackwardPass::ProcessPendingPreOpBailOutInfo(IR::Instr *const currentInstr)
24232456
Assert(bailOutInfo->bailOutOffset == preOpBailOutInstrToProcess->GetByteCodeOffset());
24242457
ProcessBailOutInfo(preOpBailOutInstrToProcess, bailOutInfo);
24252458
preOpBailOutInstrToProcess = nullptr;
2459+
2460+
// We might have removed the prev instr if it was a ByteCodeUsesInstr
2461+
// Update the prevInstr on the main loop
2462+
return currentInstr->m_prev;
24262463
}
24272464

24282465
void
@@ -2720,7 +2757,7 @@ BackwardPass::ProcessBlock(BasicBlock * block)
27202757
MarkScopeObjSymUseForStackArgOpt();
27212758
ProcessBailOnStackArgsOutOfActualsRange();
27222759

2723-
if (ProcessNoImplicitCallUses(instr) || this->ProcessBailOutInfo(instr))
2760+
if (ProcessNoImplicitCallUses(instr) || this->ProcessByteCodeUsesInstr(instr) || this->ProcessBailOutInfo(instr))
27242761
{
27252762
continue;
27262763
}
@@ -3531,7 +3568,7 @@ BackwardPass::ProcessBlock(BasicBlock * block)
35313568
}
35323569
}
35333570
#endif
3534-
ProcessPendingPreOpBailOutInfo(instr);
3571+
instrPrev = ProcessPendingPreOpBailOutInfo(instr);
35353572

35363573
#if DBG_DUMP
35373574
if (!IsCollectionPass() && IsTraceEnabled() && Js::Configuration::Global.flags.Verbose)

‎lib/Backend/BackwardPass.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,12 @@ class BackwardPass
3737
void ProcessTransfers(IR::Instr * instr);
3838
void ProcessFieldKills(IR::Instr * instr);
3939
template<typename T> void ClearBucketsOnFieldKill(IR::Instr *instr, HashTable<T> *table);
40+
StackSym* ProcessByteCodeUsesDst(IR::ByteCodeUsesInstr * byteCodeUsesInstr);
41+
const BVSparse<JitArenaAllocator>* ProcessByteCodeUsesSrcs(IR::ByteCodeUsesInstr * byteCodeUsesInstr);
42+
bool ProcessByteCodeUsesInstr(IR::Instr * instr);
4043
bool ProcessBailOutInfo(IR::Instr * instr);
4144
void ProcessBailOutInfo(IR::Instr * instr, BailOutInfo * bailOutInfo);
42-
void ProcessPendingPreOpBailOutInfo(IR::Instr *const currentInstr);
45+
IR::Instr* ProcessPendingPreOpBailOutInfo(IR::Instr *const currentInstr);
4346
void ProcessBailOutArgObj(BailOutInfo * bailOutInfo, BVSparse<JitArenaAllocator> * byteCodeUpwardExposedUsed);
4447
void ProcessBailOutConstants(BailOutInfo * bailOutInfo, BVSparse<JitArenaAllocator> * byteCodeUpwardExposedUsed, BVSparse<JitArenaAllocator>* argSymsBv);
4548
void ProcessBailOutCopyProps(BailOutInfo * bailOutInfo, BVSparse<JitArenaAllocator> * byteCodeUpwardExposedUsed, BVSparse<JitArenaAllocator>* argSymsBv);

‎lib/Backend/GlobOpt.cpp‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2670,16 +2670,6 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
26702670
this->InsertByteCodeUses(instr);
26712671
}
26722672

2673-
if (!IsLoopPrePass() && instr->HasBailOutInfo() && !this->func->IsJitInDebugMode() && !PHASE_OFF(Js::AggregateByteCodeUsesPhase, this->func))
2674-
{
2675-
// Aggregate byteCodeUpwardExposedUsed of preceding ByteCodeUses instrs with the same bytecode offset.
2676-
// This is required as different ByteCodeUses instrs may be inserted for an instr in the loop pre-pass
2677-
// and the main pass (and there may be additional instructions inserted between the two sets of
2678-
// ByteCodeUses instructions), but the Backward Pass only processes immediately preceding consecutive
2679-
// ByteCodeUses instructions before processing a pre-op bailout.
2680-
instr->AggregateByteCodeUses();
2681-
}
2682-
26832673
if (!this->IsLoopPrePass() && !isHoisted && this->IsImplicitCallBailOutCurrentlyNeeded(instr, src1Val, src2Val))
26842674
{
26852675
IR::BailOutKind kind = IR::BailOutOnImplicitCalls;

‎lib/Backend/IR.cpp‎

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,6 +1020,7 @@ void ByteCodeUsesInstr::Aggregate(ByteCodeUsesInstr * byteCodeUsesInstr)
10201020
Assert(this->m_func == byteCodeUsesInstr->m_func);
10211021
if (byteCodeUsesInstr->byteCodeUpwardExposedUsed)
10221022
{
1023+
Assert(byteCodeUsesInstr->GetDst() == nullptr);
10231024
if (this->byteCodeUpwardExposedUsed)
10241025
{
10251026
this->byteCodeUpwardExposedUsed->Or(byteCodeUsesInstr->byteCodeUpwardExposedUsed);
@@ -1036,7 +1037,7 @@ void ByteCodeUsesInstr::Aggregate(ByteCodeUsesInstr * byteCodeUsesInstr)
10361037

10371038
bool Instr::CanAggregateByteCodeUsesAcrossInstr(Instr * instr)
10381039
{
1039-
return !instr->IsBranchInstr() &&
1040+
return !instr->EndsBasicBlock() &&
10401041
instr->m_func == this->m_func &&
10411042
((instr->GetByteCodeOffset() == Js::Constants::NoByteCodeOffset) ||
10421043
(instr->GetByteCodeOffset() == this->GetByteCodeOffset()));
@@ -2871,21 +2872,6 @@ IR::Instr *Instr::GetInsertBeforeByteCodeUsesInstr()
28712872
return insertBeforeInstr;
28722873
}
28732874

2874-
IR::ByteCodeUsesInstr *
2875-
Instr::GetFirstByteCodeUsesInstrBackward()
2876-
{
2877-
IR::Instr * prevInstr = this->m_prev;
2878-
while(prevInstr && CanAggregateByteCodeUsesAcrossInstr(prevInstr))
2879-
{
2880-
if (prevInstr->IsByteCodeUsesInstr() && prevInstr->GetByteCodeOffset() == this->GetByteCodeOffset())
2881-
{
2882-
return prevInstr->AsByteCodeUsesInstr();
2883-
}
2884-
prevInstr = prevInstr->m_prev;
2885-
}
2886-
return nullptr;
2887-
}
2888-
28892875
bool
28902876
Instr::IsByteCodeUsesInstrFor(IR::Instr * instr) const
28912877
{
@@ -3395,22 +3381,6 @@ void Instr::Move(IR::Instr* insertInstr)
33953381
insertInstr->InsertBefore(this);
33963382
}
33973383

3398-
void Instr::AggregateByteCodeUses()
3399-
{
3400-
// Currently, this only aggregates byteCodeUpwardExposedUsed of those ByteCodeUses instructions
3401-
// associated with this instr which have at most a ToVar, Ld_A, Ld_I4, or a BailOnNotObject between them.
3402-
IR::ByteCodeUsesInstr * primaryByteCodeUsesInstr = this->GetFirstByteCodeUsesInstrBackward();
3403-
if (primaryByteCodeUsesInstr)
3404-
{
3405-
primaryByteCodeUsesInstr->AggregatePrecedingByteCodeUses();
3406-
if (primaryByteCodeUsesInstr != this->m_prev)
3407-
{
3408-
primaryByteCodeUsesInstr->Unlink();
3409-
this->InsertBefore(primaryByteCodeUsesInstr);
3410-
}
3411-
}
3412-
}
3413-
34143384
IR::Instr* Instr::GetBytecodeArgOutCapture()
34153385
{
34163386
Assert(this->m_opcode == Js::OpCode::ArgOut_A_Inline ||

‎lib/Backend/IR.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,6 @@ class Instr
296296
IR::Instr * GetPrevRealInstr() const;
297297
IR::Instr * GetPrevRealInstrOrLabel() const;
298298
IR::Instr * GetInsertBeforeByteCodeUsesInstr();
299-
IR::ByteCodeUsesInstr * GetFirstByteCodeUsesInstrBackward();
300299
bool IsByteCodeUsesInstrFor(IR::Instr * instr) const;
301300
IR::LabelInstr *GetOrCreateContinueLabel(const bool isHelper = false);
302301
static bool HasSymUseSrc(StackSym *sym, IR::Opnd*);
@@ -475,7 +474,6 @@ class Instr
475474
bool UsesAllFields();
476475
void MoveArgs(bool generateByteCodeCapture = false);
477476
void Move(IR::Instr* insertInstr);
478-
void AggregateByteCodeUses();
479477
private:
480478
void ClearNumber() { this->m_number = 0; }
481479
void SetNumber(uint32 number);

‎lib/Common/ConfigFlagsList.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ PHASE(All)
119119
PHASE(ValueTable)
120120
PHASE(ValueNumbering)
121121
PHASE(AVTInPrePass)
122-
PHASE(AggregateByteCodeUses)
123122
PHASE(PathDependentValues)
124123
PHASE(TrackRelativeIntBounds)
125124
PHASE(BoundCheckElimination)

0 commit comments

Comments
 (0)