Skip to content

Commit c61350e

Browse files
rajatdrajatd
committed
Refcounting captured values
1 parent 042ee40 commit c61350e

File tree

11 files changed

+109
-64
lines changed

11 files changed

+109
-64
lines changed

‎lib/Backend/BackwardPass.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,9 +1609,9 @@ BackwardPass::ProcessBailOutArgObj(BailOutInfo * bailOutInfo, BVSparse<JitArenaA
16091609
{
16101610
Assert(this->tag != Js::BackwardPhase);
16111611

1612-
if (this->globOpt->TrackArgumentsObject() && bailOutInfo->capturedValues.argObjSyms)
1612+
if (this->globOpt->TrackArgumentsObject() && bailOutInfo->capturedValues->argObjSyms)
16131613
{
1614-
FOREACH_BITSET_IN_SPARSEBV(symId, bailOutInfo->capturedValues.argObjSyms)
1614+
FOREACH_BITSET_IN_SPARSEBV(symId, bailOutInfo->capturedValues->argObjSyms)
16151615
{
16161616
if (byteCodeUpwardExposedUsed->TestAndClear(symId))
16171617
{
@@ -1646,7 +1646,7 @@ BackwardPass::ProcessBailOutConstants(BailOutInfo * bailOutInfo, BVSparse<JitAre
16461646
NEXT_SLISTBASE_ENTRY;
16471647

16481648
// Find other constants that we need to restore
1649-
FOREACH_SLISTBASE_ENTRY_EDITING(ConstantStackSymValue, value, &bailOutInfo->capturedValues.constantValues, iter)
1649+
FOREACH_SLISTBASE_ENTRY_EDITING(ConstantStackSymValue, value, &bailOutInfo->capturedValues->constantValues, iter)
16501650
{
16511651
if (byteCodeUpwardExposedUsed->TestAndClear(value.Key()->m_id) || bailoutReferencedArgSymsBv->TestAndClear(value.Key()->m_id))
16521652
{
@@ -1682,7 +1682,7 @@ BackwardPass::ProcessBailOutCopyProps(BailOutInfo * bailOutInfo, BVSparse<JitAre
16821682
BVSparse<JitArenaAllocator> * upwardExposedUses = block->upwardExposedUses;
16831683

16841684
// Find other copy prop that we need to restore
1685-
FOREACH_SLISTBASE_ENTRY_EDITING(CopyPropSyms, copyPropSyms, &bailOutInfo->capturedValues.copyPropSyms, iter)
1685+
FOREACH_SLISTBASE_ENTRY_EDITING(CopyPropSyms, copyPropSyms, &bailOutInfo->capturedValues->copyPropSyms, iter)
16861686
{
16871687
// Copy prop syms should be vars
16881688
Assert(!copyPropSyms.Key()->IsTypeSpec());

‎lib/Backend/BailOut.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@ BailOutInfo::Clear(JitArenaAllocator * allocator)
1818
{
1919
// Currently, we don't have a case where we delete bailout info after we allocated the bailout record
2020
Assert(!bailOutRecord);
21-
this->capturedValues.constantValues.Clear(allocator);
22-
this->capturedValues.copyPropSyms.Clear(allocator);
21+
if (this->capturedValues && this->capturedValues->DecrementRefCount() == 0)
22+
{
23+
this->capturedValues->constantValues.Clear(allocator);
24+
this->capturedValues->copyPropSyms.Clear(allocator);
25+
JitAdelete(allocator, this->capturedValues);
26+
}
2327
this->usedCapturedValues.constantValues.Clear(allocator);
2428
this->usedCapturedValues.copyPropSyms.Clear(allocator);
2529
if (byteCodeUpwardExposedUsed)

‎lib/Backend/BailOut.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ class BailOutInfo
4646
#if DBG
4747
wasCopied = false;
4848
#endif
49-
this->capturedValues.argObjSyms = nullptr;
49+
this->capturedValues = JitAnew(bailOutFunc->m_alloc, CapturedValues);
50+
this->capturedValues->refCount = 1;
5051
this->usedCapturedValues.argObjSyms = nullptr;
5152
}
5253
void Clear(JitArenaAllocator * allocator);
@@ -82,7 +83,7 @@ class BailOutInfo
8283
#endif
8384
uint32 bailOutOffset;
8485
BailOutRecord * bailOutRecord;
85-
CapturedValues capturedValues; // Values we know about after forward pass
86+
CapturedValues* capturedValues; // Values we know about after forward pass
8687
CapturedValues usedCapturedValues; // Values that need to be restored in the bail out
8788
BVSparse<JitArenaAllocator> * byteCodeUpwardExposedUsed; // Non-constant stack syms that needs to be restored in the bail out
8889
uint polymorphicCacheIndex;

‎lib/Backend/GlobOpt.cpp

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2763,29 +2763,7 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
27632763

27642764
if (!isHoisted && instr->HasBailOutInfo() && !this->IsLoopPrePass())
27652765
{
2766-
GlobOptBlockData * globOptData = CurrentBlockData();
2767-
globOptData->changedSyms->ClearAll();
2768-
2769-
if (!this->changedSymsAfterIncBailoutCandidate->IsEmpty())
2770-
{
2771-
//
2772-
// some symbols are changed after the values for current bailout have been
2773-
// captured (GlobOpt::CapturedValues), need to restore such symbols as changed
2774-
// for following incremental bailout construction, or we will miss capturing
2775-
// values for later bailout
2776-
//
2777-
2778-
// swap changedSyms and changedSymsAfterIncBailoutCandidate
2779-
// because both are from this->alloc
2780-
BVSparse<JitArenaAllocator> * tempBvSwap = globOptData->changedSyms;
2781-
globOptData->changedSyms = this->changedSymsAfterIncBailoutCandidate;
2782-
this->changedSymsAfterIncBailoutCandidate = tempBvSwap;
2783-
}
2784-
2785-
globOptData->capturedValues = globOptData->capturedValuesCandidate;
2786-
2787-
// null out capturedValuesCandicate to stop tracking symbols change for it
2788-
globOptData->capturedValuesCandidate = nullptr;
2766+
this->CommitCapturedValuesCandidate();
27892767
}
27902768

27912769
return instrNext;
@@ -15438,6 +15416,37 @@ GlobOptBlockData * GlobOpt::CurrentBlockData()
1543815416
return &this->currentBlock->globOptData;
1543915417
}
1544015418

15419+
void GlobOpt::CommitCapturedValuesCandidate()
15420+
{
15421+
GlobOptBlockData * globOptData = CurrentBlockData();
15422+
globOptData->changedSyms->ClearAll();
15423+
15424+
if (!this->changedSymsAfterIncBailoutCandidate->IsEmpty())
15425+
{
15426+
//
15427+
// some symbols are changed after the values for current bailout have been
15428+
// captured (GlobOpt::CapturedValues), need to restore such symbols as changed
15429+
// for following incremental bailout construction, or we will miss capturing
15430+
// values for later bailout
15431+
//
15432+
15433+
// swap changedSyms and changedSymsAfterIncBailoutCandidate
15434+
// because both are from this->alloc
15435+
BVSparse<JitArenaAllocator> * tempBvSwap = globOptData->changedSyms;
15436+
globOptData->changedSyms = this->changedSymsAfterIncBailoutCandidate;
15437+
this->changedSymsAfterIncBailoutCandidate = tempBvSwap;
15438+
}
15439+
15440+
if (globOptData->capturedValues)
15441+
{
15442+
globOptData->capturedValues->DecrementRefCount();
15443+
}
15444+
globOptData->capturedValues = globOptData->capturedValuesCandidate;
15445+
15446+
// null out capturedValuesCandidate to stop tracking symbols change for it
15447+
globOptData->capturedValuesCandidate = nullptr;
15448+
}
15449+
1544115450
bool
1544215451
GlobOpt::IsOperationThatLikelyKillsJsArraysWithNoMissingValues(IR::Instr *const instr)
1544315452
{
@@ -16750,7 +16759,7 @@ GlobOpt::OptHoistInvariant(
1675016759
EnsureBailTarget(loop);
1675116760

1675216761
// Copy bailout info of loop top.
16753-
instr->ReplaceBailOutInfo(loop->bailOutInfo, this->currentBlock);
16762+
instr->ReplaceBailOutInfo(loop->bailOutInfo);
1675416763
}
1675516764

1675616765
if(!dst)

‎lib/Backend/GlobOpt.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,7 @@ class GlobOpt
701701

702702
GlobOptBlockData const * CurrentBlockData() const;
703703
GlobOptBlockData * CurrentBlockData();
704+
void CommitCapturedValuesCandidate();
704705

705706
private:
706707
bool IsOperationThatLikelyKillsJsArraysWithNoMissingValues(IR::Instr *const instr);

‎lib/Backend/GlobOptBailOut.cpp

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -253,18 +253,23 @@ GlobOpt::CaptureValues(BasicBlock *block, BailOutInfo * bailOutInfo)
253253

254254
// attach capturedValues to bailOutInfo
255255

256-
bailOutInfo->capturedValues.constantValues.Clear(this->func->m_alloc);
257-
bailOutConstValuesIter.SetNext(&bailOutInfo->capturedValues.constantValues);
258-
bailOutInfo->capturedValues.constantValues = capturedValues.constantValues;
256+
bailOutInfo->capturedValues->constantValues.Clear(this->func->m_alloc);
257+
bailOutConstValuesIter.SetNext(&bailOutInfo->capturedValues->constantValues);
258+
bailOutInfo->capturedValues->constantValues = capturedValues.constantValues;
259259

260-
bailOutInfo->capturedValues.copyPropSyms.Clear(this->func->m_alloc);
261-
bailOutCopySymsIter.SetNext(&bailOutInfo->capturedValues.copyPropSyms);
262-
bailOutInfo->capturedValues.copyPropSyms = capturedValues.copyPropSyms;
260+
bailOutInfo->capturedValues->copyPropSyms.Clear(this->func->m_alloc);
261+
bailOutCopySymsIter.SetNext(&bailOutInfo->capturedValues->copyPropSyms);
262+
bailOutInfo->capturedValues->copyPropSyms = capturedValues.copyPropSyms;
263263

264264
if (!PHASE_OFF(Js::IncrementalBailoutPhase, func))
265265
{
266266
// cache the pointer of current bailout as potential baseline for later bailout in this block
267-
block->globOptData.capturedValuesCandidate = &bailOutInfo->capturedValues;
267+
if (block->globOptData.capturedValuesCandidate)
268+
{
269+
block->globOptData.capturedValuesCandidate->DecrementRefCount();
270+
}
271+
block->globOptData.capturedValuesCandidate = bailOutInfo->capturedValues;
272+
block->globOptData.capturedValuesCandidate->IncrementRefCount();
268273

269274
// reset changed syms to track symbols change after the above captured values candidate
270275
this->changedSymsAfterIncBailoutCandidate->ClearAll();
@@ -283,12 +288,12 @@ GlobOpt::CaptureArguments(BasicBlock *block, BailOutInfo * bailOutInfo, JitArena
283288
continue;
284289
}
285290

286-
if (!bailOutInfo->capturedValues.argObjSyms)
291+
if (!bailOutInfo->capturedValues->argObjSyms)
287292
{
288-
bailOutInfo->capturedValues.argObjSyms = JitAnew(allocator, BVSparse<JitArenaAllocator>, allocator);
293+
bailOutInfo->capturedValues->argObjSyms = JitAnew(allocator, BVSparse<JitArenaAllocator>, allocator);
289294
}
290295

291-
bailOutInfo->capturedValues.argObjSyms->Set(id);
296+
bailOutInfo->capturedValues->argObjSyms->Set(id);
292297
// Add to BailOutInfo
293298
}
294299
NEXT_BITSET_IN_SPARSEBV

‎lib/Backend/GlobOptBlockData.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,10 @@ GlobOptBlockData::ReuseBlockData(GlobOptBlockData *fromData)
144144

145145
this->changedSyms = fromData->changedSyms;
146146
this->capturedValues = fromData->capturedValues;
147+
if (this->capturedValues)
148+
{
149+
this->capturedValues->IncrementRefCount();
150+
}
147151

148152
this->OnDataReused(fromData);
149153
}
@@ -254,6 +258,11 @@ GlobOptBlockData::DeleteBlockData()
254258
JitAdelete(alloc, this->changedSyms);
255259
this->changedSyms = nullptr;
256260

261+
if (this->capturedValues && this->capturedValues->DecrementRefCount() == 0)
262+
{
263+
JitAdelete(this->curFunc->m_alloc, this->capturedValues);
264+
this->capturedValues = nullptr;
265+
}
257266
this->OnDataDeleted();
258267
}
259268

@@ -367,7 +376,11 @@ void GlobOptBlockData::CloneBlockData(BasicBlock *const toBlockContext, BasicBlo
367376
this->changedSyms = JitAnew(alloc, BVSparse<JitArenaAllocator>, alloc);
368377
this->changedSyms->Copy(fromData->changedSyms);
369378
this->capturedValues = fromData->capturedValues;
370-
379+
if (this->capturedValues)
380+
{
381+
this->capturedValues->IncrementRefCount();
382+
}
383+
371384
Assert(fromData->HasData());
372385
this->OnDataInitialized(alloc);
373386
}
@@ -482,7 +495,10 @@ GlobOptBlockData::MergeBlockData(
482495
if (this->capturedValues == nullptr)
483496
{
484497
this->capturedValues = fromData->capturedValues;
485-
498+
if (this->capturedValues)
499+
{
500+
this->capturedValues->IncrementRefCount();
501+
}
486502
}
487503
else
488504
{

‎lib/Backend/IR.cpp

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,7 +1154,7 @@ Instr::UnlinkBailOutInfo()
11541154
}
11551155

11561156
void
1157-
Instr::ReplaceBailOutInfo(BailOutInfo *newBailOutInfo, BasicBlock * block)
1157+
Instr::ReplaceBailOutInfo(BailOutInfo *newBailOutInfo)
11581158
{
11591159
BailOutInfo *oldBailOutInfo = nullptr;
11601160

@@ -1186,13 +1186,8 @@ Instr::ReplaceBailOutInfo(BailOutInfo *newBailOutInfo, BasicBlock * block)
11861186
if (oldBailOutInfo->bailOutInstr == this)
11871187
{
11881188
JitArenaAllocator * alloc = this->m_func->m_alloc;
1189-
// If the oldBailOutInfo's captured values were cached on the globopt-block-data, don't
1190-
// delete the old bailout info. It will eventually be freed when we're done jitting this function.
1191-
if (!(block && block->globOptData.capturedValuesCandidate == &oldBailOutInfo->capturedValues))
1192-
{
1193-
oldBailOutInfo->Clear(alloc);
1194-
JitAdelete(alloc, oldBailOutInfo);
1195-
}
1189+
oldBailOutInfo->Clear(alloc);
1190+
JitAdelete(alloc, oldBailOutInfo);
11961191
}
11971192

11981193
return;
@@ -3148,7 +3143,7 @@ Instr::ConvertToBailOutInstr(BailOutInfo * bailOutInfo, IR::BailOutKind kind, bo
31483143
this->SetBailOutKind_NoAssert(kind);
31493144

31503145
// Clear old (aux) info and set to the new bailOutInfo.
3151-
this->ReplaceBailOutInfo(bailOutInfo, nullptr);
3146+
this->ReplaceBailOutInfo(bailOutInfo);
31523147
bailOutInfo->bailOutInstr = this;
31533148
this->hasBailOutInfo = true;
31543149

‎lib/Backend/IR.h

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,29 @@ struct CapturedValues
2626
SListBase<ConstantStackSymValue> constantValues; // Captured constant values during glob opt
2727
SListBase<CopyPropSyms> copyPropSyms; // Captured copy prop values during glob opt
2828
BVSparse<JitArenaAllocator> * argObjSyms; // Captured arg object symbols during glob opt
29+
uint refCount;
2930

30-
31-
CapturedValues() : argObjSyms(nullptr) {}
31+
CapturedValues() : argObjSyms(nullptr), refCount(0) {}
3232
~CapturedValues()
3333
{
3434
// Reset SListBase to be exception safe. Captured values are from GlobOpt->func->alloc
3535
// in normal case the 2 SListBase are empty so no Clear needed, also no need to Clear in exception case
3636
constantValues.Reset();
3737
copyPropSyms.Reset();
38+
argObjSyms = nullptr;
39+
refCount = 0;
40+
}
41+
42+
uint DecrementRefCount()
43+
{
44+
Assert(refCount != 0);
45+
return --refCount;
46+
}
47+
48+
void IncrementRefCount()
49+
{
50+
Assert(refCount > 0);
51+
refCount++;
3852
}
3953
};
4054

@@ -317,7 +331,7 @@ class Instr
317331

318332
BailOutInfo * GetBailOutInfo() const;
319333
BailOutInfo * UnlinkBailOutInfo();
320-
void ReplaceBailOutInfo(BailOutInfo *newBailOutInfo, BasicBlock * block);
334+
void ReplaceBailOutInfo(BailOutInfo *newBailOutInfo);
321335
IR::Instr * ShareBailOut();
322336
BailOutKind GetBailOutKind() const;
323337
BailOutKind GetBailOutKindNoBits() const;

‎lib/Backend/amd64/LinearScanMD.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -368,8 +368,8 @@ LinearScanMD::GenerateBailInForGeneratorYield(IR::Instr * resumeLabelInstr, Bail
368368
IR::Instr * instrInsertStackSym = instrAfter;
369369
IR::Instr * instrInsertRegSym = instrAfter;
370370

371-
Assert(bailOutInfo->capturedValues.constantValues.Empty());
372-
Assert(bailOutInfo->capturedValues.copyPropSyms.Empty());
371+
Assert(bailOutInfo->capturedValues->constantValues.Empty());
372+
Assert(bailOutInfo->capturedValues->copyPropSyms.Empty());
373373
Assert(bailOutInfo->liveLosslessInt32Syms->IsEmpty());
374374
Assert(bailOutInfo->liveFloat64Syms->IsEmpty());
375375

@@ -441,9 +441,9 @@ LinearScanMD::GenerateBailInForGeneratorYield(IR::Instr * resumeLabelInstr, Bail
441441
}
442442
NEXT_BITSET_IN_SPARSEBV;
443443

444-
if (bailOutInfo->capturedValues.argObjSyms)
444+
if (bailOutInfo->capturedValues->argObjSyms)
445445
{
446-
FOREACH_BITSET_IN_SPARSEBV(symId, bailOutInfo->capturedValues.argObjSyms)
446+
FOREACH_BITSET_IN_SPARSEBV(symId, bailOutInfo->capturedValues->argObjSyms)
447447
{
448448
StackSym* stackSym = this->func->m_symTable->FindStackSym(symId);
449449
restoreSymFn(stackSym->GetByteCodeRegSlot(), stackSym);

‎lib/Backend/i386/LinearScanMD.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,8 @@ LinearScanMD::GenerateBailInForGeneratorYield(IR::Instr * resumeLabelInstr, Bail
250250
IR::Instr * instrInsertStackSym = instrAfter;
251251
IR::Instr * instrInsertRegSym = instrAfter;
252252

253-
Assert(bailOutInfo->capturedValues.constantValues.Empty());
254-
Assert(bailOutInfo->capturedValues.copyPropSyms.Empty());
253+
Assert(bailOutInfo->capturedValues->constantValues.Empty());
254+
Assert(bailOutInfo->capturedValues->copyPropSyms.Empty());
255255

256256
auto restoreSymFn = [this, &eaxRegOpnd, &ecxRegOpnd, &eaxRestoreInstr, &instrInsertStackSym, &instrInsertRegSym](SymID symId)
257257
{
@@ -327,9 +327,9 @@ LinearScanMD::GenerateBailInForGeneratorYield(IR::Instr * resumeLabelInstr, Bail
327327
}
328328
NEXT_BITSET_IN_SPARSEBV;
329329

330-
if (bailOutInfo->capturedValues.argObjSyms)
330+
if (bailOutInfo->capturedValues->argObjSyms)
331331
{
332-
FOREACH_BITSET_IN_SPARSEBV(symId, bailOutInfo->capturedValues.argObjSyms)
332+
FOREACH_BITSET_IN_SPARSEBV(symId, bailOutInfo->capturedValues->argObjSyms)
333333
{
334334
restoreSymFn(symId);
335335
}

0 commit comments

Comments
 (0)