Skip to content

Commit 706d812

Browse files
rajatdrajatd
committed
[MERGE chakra-core#4447 @rajatd] Fixing incremental capturing of values (for bailout) in GlobOpt
Merge pull request chakra-core#4447 from rajatd:capturedValues Due to multiple issues with incremental capturing of values in globopt, we were never capturing values incrementally across blocks. This PR fixes those issues and makes capturing values across blocks incremental (for real :) ). `GlobOptBlockData.cpp:` Fixes to propagate captured values to the successor in case of a single predecessor and to merge correctly the values captured on all the incoming edges, i.e., predecessors `GlobOptBailout.cpp:` Refactoring the code for incremental value capture and fixing the case when there were captured syms which were unprocessed after going over all the syms which were changed since the last time we captured values for bailout. Gives a nice win in Octane on arm64 (fyi @agarwal-sandeep @sigatrev @Penguinwizzard @aaronsgiles): ``` Octane Left score Right score ∆ Score ∆ Score % Comment ---------------- --------------- --------------- ------- --------- --------------- Box2d 6419.45 ±0.23% 7029.38 ±1.53% 609.92 9.50% Improved Code-load 8963.50 ±0.91% 8906.50 ±0.47% -57.00 -0.64% Crypto 12125.75 ±0.86% 12234.00 ±0.43% 108.25 0.89% Deltablue 7695.00 ±0.24% 7735.27 ±0.69% 40.27 0.52% Earley-boyer 10561.20 ±0.23% 10753.70 ±0.75% 192.50 1.82% Gbemu 17752.67 ±0.71% 17980.00 ±0.54% 227.33 1.28% Mandreel 10558.83 ±0.48% 10547.43 ±0.32% -11.40 -0.11% Mandreel latency 29307.17 ±0.87% 29892.50 ±0.94% 585.33 2.00% Navier-stokes 14999.00 ±0.90% 15046.50 ±0.88% 47.50 0.32% Pdfjs 7300.00 ±0.33% 7311.83 ±0.20% 11.83 0.16% Raytrace 15915.00 ±0.13% 16177.00 ±0.95% 262.00 1.65% Regexp 1529.63 ±0.23% 1690.67 ±0.20% 161.04 10.53% Improved Richards 8593.67 ±0.94% 8702.00 ±0.98% 108.33 1.26% Splay 10626.91 ±0.23% 10881.27 ±0.17% 254.36 2.39% Improved Splay latency 22611.63 ±2.99% 24481.75 ±2.04% 1870.12 8.27% Likely improved Typescript 16565.17 ±0.15% 16412.17 ±0.81% -153.00 -0.92% Zlib 19689.00 ±0.36% 19647.50 ±0.91% -41.50 -0.21% ---------------- --------------- --------------- ------- --------- --------------- Total 11065.60 ±0.64% 11311.37 ±0.75% 245.77 2.22% Likely improved ``` splay wins are probably not real wins and box2d numbers weren't consistent, but consistently better, regexp wins are definitely real.
2 parents 330f746 + bb46712 commit 706d812

7 files changed

Lines changed: 199 additions & 106 deletions

File tree

‎lib/Backend/BackwardPass.cpp‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,36 +1740,36 @@ BackwardPass::ProcessBailOutCopyProps(BailOutInfo * bailOutInfo, BVSparse<JitAre
17401740
StackSym * typeSpecSym = nullptr;
17411741
auto findTypeSpecSym = [&]()
17421742
{
1743-
if (bailOutInfo->liveLosslessInt32Syms->Test(symId))
1744-
{
1745-
// Var version of the sym is not live, use the int32 version
1746-
int32StackSym = stackSym->GetInt32EquivSym(nullptr);
1743+
if (bailOutInfo->liveLosslessInt32Syms->Test(symId))
1744+
{
1745+
// Var version of the sym is not live, use the int32 version
1746+
int32StackSym = stackSym->GetInt32EquivSym(nullptr);
17471747
typeSpecSym = int32StackSym;
1748-
Assert(int32StackSym);
1749-
}
1750-
else if(bailOutInfo->liveFloat64Syms->Test(symId))
1751-
{
1752-
// Var/int32 version of the sym is not live, use the float64 version
1753-
float64StackSym = stackSym->GetFloat64EquivSym(nullptr);
1748+
Assert(int32StackSym);
1749+
}
1750+
else if(bailOutInfo->liveFloat64Syms->Test(symId))
1751+
{
1752+
// Var/int32 version of the sym is not live, use the float64 version
1753+
float64StackSym = stackSym->GetFloat64EquivSym(nullptr);
17541754
typeSpecSym = float64StackSym;
1755-
Assert(float64StackSym);
1756-
}
1755+
Assert(float64StackSym);
1756+
}
17571757
#ifdef ENABLE_SIMDJS
1758-
// SIMD_JS
1759-
else if (bailOutInfo->liveSimd128F4Syms->Test(symId))
1760-
{
1761-
simd128StackSym = stackSym->GetSimd128F4EquivSym(nullptr);
1758+
// SIMD_JS
1759+
else if (bailOutInfo->liveSimd128F4Syms->Test(symId))
1760+
{
1761+
simd128StackSym = stackSym->GetSimd128F4EquivSym(nullptr);
17621762
typeSpecSym = simd128StackSym;
1763-
}
1764-
else if (bailOutInfo->liveSimd128I4Syms->Test(symId))
1765-
{
1766-
simd128StackSym = stackSym->GetSimd128I4EquivSym(nullptr);
1763+
}
1764+
else if (bailOutInfo->liveSimd128I4Syms->Test(symId))
1765+
{
1766+
simd128StackSym = stackSym->GetSimd128I4EquivSym(nullptr);
17671767
typeSpecSym = simd128StackSym;
1768-
}
1768+
}
17691769
#endif
1770-
else
1771-
{
1772-
Assert(bailOutInfo->liveVarSyms->Test(symId));
1770+
else
1771+
{
1772+
Assert(bailOutInfo->liveVarSyms->Test(symId));
17731773
typeSpecSym = stackSym;
17741774
}
17751775
};

‎lib/Backend/GlobOpt.cpp‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2761,7 +2761,7 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
27612761
}
27622762
}
27632763

2764-
if (instr->HasBailOutInfo() && !this->IsLoopPrePass())
2764+
if (!isHoisted && instr->HasBailOutInfo() && !this->IsLoopPrePass())
27652765
{
27662766
GlobOptBlockData * globOptData = CurrentBlockData();
27672767
globOptData->changedSyms->ClearAll();
@@ -16733,12 +16733,7 @@ GlobOpt::OptHoistInvariant(
1673316733
EnsureBailTarget(loop);
1673416734

1673516735
// Copy bailout info of loop top.
16736-
if (instr->ReplaceBailOutInfo(loop->bailOutInfo))
16737-
{
16738-
// if the old bailout is deleted, reset capturedvalues cached in block
16739-
block->globOptData.capturedValues = nullptr;
16740-
block->globOptData.capturedValuesCandidate = nullptr;
16741-
}
16736+
instr->ReplaceBailOutInfo(loop->bailOutInfo);
1674216737
}
1674316738

1674416739
if(!dst)

‎lib/Backend/GlobOptBailOut.cpp‎

Lines changed: 84 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -92,119 +92,135 @@ GlobOpt::CaptureValuesIncremental(BasicBlock * block,
9292

9393
FOREACH_BITSET_IN_SPARSEBV(symId, block->globOptData.changedSyms)
9494
{
95-
Sym * sym = hasConstValue ? iterConst.Data().Key() : nullptr;
9695
Value * val = nullptr;
97-
HashBucket<Sym *, Value *> * symIdBucket = nullptr;
9896

99-
// copy unchanged sym to new capturedValues
100-
while (sym && sym->m_id < symId)
97+
// First process all unchanged syms with m_id < symId. Then, recapture the current changed sym.
98+
99+
// copy unchanged const sym to new capturedValues
100+
Sym * constSym = hasConstValue ? iterConst.Data().Key() : nullptr;
101+
while (constSym && constSym->m_id < symId)
101102
{
102-
Assert(sym->IsStackSym());
103-
if (!sym->AsStackSym()->HasArgSlotNum())
103+
Assert(constSym->IsStackSym());
104+
if (!constSym->AsStackSym()->HasArgSlotNum())
104105
{
105-
bailOutConstValuesIter.InsertNodeBefore(this->func->m_alloc, sym->AsStackSym(), iterConst.Data().Value());
106+
bailOutConstValuesIter.InsertNodeBefore(this->func->m_alloc, constSym->AsStackSym(), iterConst.Data().Value());
106107
}
107108

108109
hasConstValue = iterConst.Next();
109-
sym = hasConstValue ? iterConst.Data().Key() : nullptr;
110+
constSym = hasConstValue ? iterConst.Data().Key() : nullptr;
110111
}
111-
if (sym && sym->m_id == symId)
112+
if (constSym && constSym->m_id == symId)
112113
{
113114
hasConstValue = iterConst.Next();
114115
}
115-
if (symId != Js::Constants::InvalidSymID)
116-
{
117-
// recapture changed constant sym
118116

119-
symIdBucket = block->globOptData.symToValueMap->GetBucket(symId);
120-
if (symIdBucket == nullptr)
121-
{
122-
continue;
123-
}
124-
125-
Sym * symIdSym = symIdBucket->value;
126-
Assert(symIdSym->IsStackSym() && (symIdSym->AsStackSym()->HasByteCodeRegSlot() || symIdSym->AsStackSym()->HasArgSlotNum()));
127-
128-
val = symIdBucket->element;
129-
ValueInfo* valueInfo = val->GetValueInfo();
130-
131-
if (valueInfo->GetSymStore() != nullptr)
132-
{
133-
int32 intConstValue;
134-
BailoutConstantValue constValue;
135-
136-
if (valueInfo->TryGetIntConstantValue(&intConstValue))
137-
{
138-
constValue.InitIntConstValue(intConstValue);
139-
bailOutConstValuesIter.InsertNodeBefore(this->func->m_alloc, symIdSym->AsStackSym(), constValue);
140-
141-
continue;
142-
}
143-
else if(valueInfo->IsVarConstant())
144-
{
145-
constValue.InitVarConstValue(valueInfo->AsVarConstant()->VarValue());
146-
bailOutConstValuesIter.InsertNodeBefore(this->func->m_alloc, symIdSym->AsStackSym(), constValue);
147-
148-
continue;
149-
}
150-
}
151-
else if (!valueInfo->HasIntConstantValue())
152-
{
153-
continue;
154-
}
155-
}
156-
157-
sym = hasCopyPropSym ? iterCopyPropSym.Data().Key() : nullptr;
158-
159-
// process unchanged sym, but copy sym might have changed
160-
while (sym && sym->m_id < symId)
117+
// process unchanged sym; copy-prop sym might have changed
118+
Sym * capturedSym = hasCopyPropSym ? iterCopyPropSym.Data().Key() : nullptr;
119+
while (capturedSym && capturedSym->m_id < symId)
161120
{
162-
StackSym * copyPropSym = iterCopyPropSym.Data().Value();
121+
StackSym * capturedCopyPropSym = iterCopyPropSym.Data().Value();
163122

164-
Assert(sym->IsStackSym());
123+
Assert(capturedSym->IsStackSym());
165124

166-
if (!block->globOptData.changedSyms->Test(copyPropSym->m_id))
125+
if (!block->globOptData.changedSyms->Test(capturedCopyPropSym->m_id))
167126
{
168-
if (!sym->AsStackSym()->HasArgSlotNum())
127+
if (!capturedSym->AsStackSym()->HasArgSlotNum())
169128
{
170-
bailOutCopySymsIter.InsertNodeBefore(this->func->m_alloc, sym->AsStackSym(), copyPropSym);
129+
bailOutCopySymsIter.InsertNodeBefore(this->func->m_alloc, capturedSym->AsStackSym(), capturedCopyPropSym);
171130
}
172131
}
173132
else
174133
{
175-
if (!sym->AsStackSym()->HasArgSlotNum())
134+
if (!capturedSym->AsStackSym()->HasArgSlotNum())
176135
{
177-
val = this->currentBlock->globOptData.FindValue(sym);
136+
val = this->currentBlock->globOptData.FindValue(capturedSym);
178137
if (val != nullptr)
179138
{
180-
CaptureCopyPropValue(block, sym, val, bailOutCopySymsIter);
139+
CaptureCopyPropValue(block, capturedSym, val, bailOutCopySymsIter);
181140
}
182141
}
183142
}
184143

185144
hasCopyPropSym = iterCopyPropSym.Next();
186-
sym = hasCopyPropSym ? iterCopyPropSym.Data().Key() : nullptr;
145+
capturedSym = hasCopyPropSym ? iterCopyPropSym.Data().Key() : nullptr;
187146
}
188-
if (sym && sym->m_id == symId)
147+
if (capturedSym && capturedSym->m_id == symId)
189148
{
190149
hasCopyPropSym = iterCopyPropSym.Next();
191150
}
151+
152+
// recapture changed sym
153+
HashBucket<Sym *, Value *> * symIdBucket = nullptr;
192154
if (symId != Js::Constants::InvalidSymID)
193155
{
194-
// recapture changed copy prop sym
195156
symIdBucket = block->globOptData.symToValueMap->GetBucket(symId);
196157
if (symIdBucket != nullptr)
197158
{
198159
Sym * symIdSym = symIdBucket->value;
199-
val = this->currentBlock->globOptData.FindValue(symIdSym);
200-
if (val != nullptr)
160+
Assert(symIdSym->IsStackSym() && (symIdSym->AsStackSym()->HasByteCodeRegSlot() || symIdSym->AsStackSym()->HasArgSlotNum()));
161+
162+
val = symIdBucket->element;
163+
Assert(val);
164+
ValueInfo* valueInfo = val->GetValueInfo();
165+
166+
if (valueInfo->GetSymStore() != nullptr)
201167
{
202-
CaptureCopyPropValue(block, symIdSym, val, bailOutCopySymsIter);
168+
int32 intConstValue;
169+
BailoutConstantValue constValue;
170+
171+
if (valueInfo->TryGetIntConstantValue(&intConstValue))
172+
{
173+
constValue.InitIntConstValue(intConstValue);
174+
bailOutConstValuesIter.InsertNodeBefore(this->func->m_alloc, symIdSym->AsStackSym(), constValue);
175+
}
176+
else if (valueInfo->IsVarConstant())
177+
{
178+
constValue.InitVarConstValue(valueInfo->AsVarConstant()->VarValue());
179+
bailOutConstValuesIter.InsertNodeBefore(this->func->m_alloc, symIdSym->AsStackSym(), constValue);
180+
}
181+
else
182+
{
183+
CaptureCopyPropValue(block, symIdSym, val, bailOutCopySymsIter);
184+
}
203185
}
204186
}
205187
}
206188
}
207189
NEXT_BITSET_IN_SPARSEBV
190+
191+
// If, after going over the set of changed syms since the last time we captured values,
192+
// there are remaining unprocessed entries in the current captured values set,
193+
// they can simply be copied over to the new bailout info.
194+
while (hasConstValue)
195+
{
196+
Sym * constSym = iterConst.Data().Key();
197+
Assert(constSym->IsStackSym());
198+
Assert(!block->globOptData.changedSyms->Test(constSym->m_id));
199+
200+
if (!constSym->AsStackSym()->HasArgSlotNum())
201+
{
202+
bailOutConstValuesIter.InsertNodeBefore(this->func->m_alloc, constSym->AsStackSym(), iterConst.Data().Value());
203+
}
204+
205+
hasConstValue = iterConst.Next();
206+
}
207+
208+
while (hasCopyPropSym)
209+
{
210+
Sym * capturedSym = iterCopyPropSym.Data().Key();
211+
StackSym * capturedCopyPropSym = iterCopyPropSym.Data().Value();
212+
213+
Assert(capturedSym->IsStackSym());
214+
Assert(!block->globOptData.changedSyms->Test(capturedSym->m_id) &&
215+
!block->globOptData.changedSyms->Test(capturedCopyPropSym->m_id));
216+
217+
if (!capturedSym->AsStackSym()->HasArgSlotNum())
218+
{
219+
bailOutCopySymsIter.InsertNodeBefore(this->func->m_alloc, capturedSym->AsStackSym(), capturedCopyPropSym);
220+
}
221+
222+
hasCopyPropSym = iterCopyPropSym.Next();
223+
}
208224
}
209225

210226

‎lib/Backend/GlobOptBlockData.cpp‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ GlobOptBlockData::ReuseBlockData(GlobOptBlockData *fromData)
143143
this->stackLiteralInitFldDataMap = fromData->stackLiteralInitFldDataMap;
144144

145145
this->changedSyms = fromData->changedSyms;
146-
this->changedSyms->ClearAll();
146+
this->capturedValues = fromData->capturedValues;
147147

148148
this->OnDataReused(fromData);
149149
}
@@ -184,6 +184,7 @@ GlobOptBlockData::CopyBlockData(GlobOptBlockData *fromData)
184184
this->hasCSECandidates = fromData->hasCSECandidates;
185185

186186
this->changedSyms = fromData->changedSyms;
187+
this->capturedValues = fromData->capturedValues;
187188

188189
this->stackLiteralInitFldDataMap = fromData->stackLiteralInitFldDataMap;
189190
this->OnDataReused(fromData);
@@ -365,7 +366,8 @@ void GlobOptBlockData::CloneBlockData(BasicBlock *const toBlockContext, BasicBlo
365366

366367
this->changedSyms = JitAnew(alloc, BVSparse<JitArenaAllocator>, alloc);
367368
this->changedSyms->Copy(fromData->changedSyms);
368-
369+
this->capturedValues = fromData->capturedValues;
370+
369371
Assert(fromData->HasData());
370372
this->OnDataInitialized(alloc);
371373
}
@@ -476,10 +478,11 @@ GlobOptBlockData::MergeBlockData(
476478
this->isTempSrc->And(fromData->isTempSrc);
477479
this->hasCSECandidates &= fromData->hasCSECandidates;
478480

481+
this->changedSyms->Or(fromData->changedSyms);
479482
if (this->capturedValues == nullptr)
480483
{
481484
this->capturedValues = fromData->capturedValues;
482-
this->changedSyms->Or(fromData->changedSyms);
485+
483486
}
484487
else
485488
{
@@ -488,6 +491,7 @@ GlobOptBlockData::MergeBlockData(
488491
fromData->capturedValues == nullptr ? nullptr : &fromData->capturedValues->constantValues,
489492
[&](ConstantStackSymValue * symValueFrom, ConstantStackSymValue * symValueTo)
490493
{
494+
Assert(symValueFrom->Key()->m_id == symValueTo->Key()->m_id);
491495
return symValueFrom->Value().IsEqual(symValueTo->Value());
492496
});
493497

@@ -496,12 +500,12 @@ GlobOptBlockData::MergeBlockData(
496500
fromData->capturedValues == nullptr ? nullptr : &fromData->capturedValues->copyPropSyms,
497501
[&](CopyPropSyms * copyPropSymFrom, CopyPropSyms * copyPropSymTo)
498502
{
503+
Assert(copyPropSymFrom->Key()->m_id == copyPropSymTo->Key()->m_id);
499504
if (copyPropSymFrom->Value()->m_id == copyPropSymTo->Value()->m_id)
500505
{
501-
Value * val = fromData->FindValue(copyPropSymFrom->Key());
502-
Value * copyVal = fromData->FindValue(copyPropSymTo->Key());
503-
return (val != nullptr && copyVal != nullptr &&
504-
val->GetValueNumber() == copyVal->GetValueNumber());
506+
Value * fromVal = fromData->FindValue(copyPropSymFrom->Key());
507+
Value * toVal = this->FindValue(copyPropSymFrom->Key());
508+
return fromVal && toVal && fromVal->IsEqualTo(toVal);
505509
}
506510
return false;
507511
});

‎lib/Backend/IR.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ struct CapturedValues
2727
SListBase<CopyPropSyms> copyPropSyms; // Captured copy prop values during glob opt
2828
BVSparse<JitArenaAllocator> * argObjSyms; // Captured arg object symbols during glob opt
2929

30+
31+
CapturedValues() : argObjSyms(nullptr) {}
3032
~CapturedValues()
3133
{
3234
// Reset SListBase to be exception safe. Captured values are from GlobOpt->func->alloc

0 commit comments

Comments
 (0)