Skip to content

Commit 9e54d66

Browse files
nhat-nguyennhat-nguyen
committed
[MERGE #6226 @nhat-nguyen] Enable globopt for generator functions
Merge pull request #6226 from nhat-nguyen:globopt + Bail-in points now rely on `upwardExposed` bitvector to compute the right symbol to restore to + Disallow hoisting loop invariants if the loop contains yield + Disallow certain optimizations across yield points + Move `BailOutForElidedYield` as part of the jump table + Disable auto-profiling interpreter mode for generators + Add fast path that loads data directly from ResumeYieldData for OP_ResumeYield I will put other runtime improvements in another PR
2 parents 9d90278 + 726c431 commit 9e54d66

30 files changed

+763
-279
lines changed

‎bin/NativeTests/FunctionExecutionTest.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ namespace Js
127127
FunctionEntryPointInfo* GetDefaultFunctionEntryPointInfo() { return &defaultInfo; }
128128
FunctionEntryPointInfo *GetSimpleJitEntryPointInfo() { return &simpleInfo; }
129129
void TraceExecutionMode(const char *const eventDescription = nullptr) const { UNREFERENCED_PARAMETER(eventDescription); }
130+
// Dummy implementation to match the real FunctionBody's method
131+
bool SkipAutoProfileForCoroutine() const { return false; }
130132

131133
FunctionBody(bool interpreterProfile, bool interpreterAutoProfile, bool simpleJit):
132134
doInterpreterProfile(interpreterProfile),

‎lib/Backend/BackwardPass.cpp

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,9 @@ BackwardPass::DoMarkTempNumbers() const
9090
bool
9191
BackwardPass::SatisfyMarkTempObjectsConditions() const {
9292
return !PHASE_OFF(Js::MarkTempPhase, this->func) &&
93-
!PHASE_OFF(Js::MarkTempObjectPhase, this->func) &&
94-
func->DoGlobOpt() && func->GetHasTempObjectProducingInstr() &&
95-
!func->IsJitInDebugMode() &&
96-
func->DoGlobOptsForGeneratorFunc();
93+
!PHASE_OFF(Js::MarkTempObjectPhase, this->func) &&
94+
func->DoGlobOpt() && func->GetHasTempObjectProducingInstr() &&
95+
!func->IsJitInDebugMode();
9796

9897
// Why MarkTempObject is disabled under debugger:
9998
// We add 'identified so far dead non-temp locals' to byteCodeUpwardExposedUsed in ProcessBailOutInfo,
@@ -156,8 +155,7 @@ BackwardPass::DoDeadStore(Func* func, StackSym* sym)
156155
// Dead store is disabled under debugger for non-temp local vars.
157156
return
158157
DoDeadStore(func) &&
159-
!(func->IsJitInDebugMode() && sym->HasByteCodeRegSlot() && func->IsNonTempLocalVar(sym->GetByteCodeRegSlot())) &&
160-
func->DoGlobOptsForGeneratorFunc();
158+
!(func->IsJitInDebugMode() && sym->HasByteCodeRegSlot() && func->IsNonTempLocalVar(sym->GetByteCodeRegSlot()));
161159
}
162160

163161
bool
@@ -168,8 +166,7 @@ BackwardPass::DoTrackNegativeZero() const
168166
!PHASE_OFF(Js::TrackNegativeZeroPhase, func) &&
169167
func->DoGlobOpt() &&
170168
!IsPrePass() &&
171-
!func->IsJitInDebugMode() &&
172-
func->DoGlobOptsForGeneratorFunc();
169+
!func->IsJitInDebugMode();
173170
}
174171

175172
bool
@@ -181,8 +178,7 @@ BackwardPass::DoTrackBitOpsOrNumber() const
181178
tag == Js::BackwardPhase &&
182179
func->DoGlobOpt() &&
183180
!IsPrePass() &&
184-
!func->IsJitInDebugMode() &&
185-
func->DoGlobOptsForGeneratorFunc();
181+
!func->IsJitInDebugMode();
186182
#else
187183
return false;
188184
#endif
@@ -197,8 +193,7 @@ BackwardPass::DoTrackIntOverflow() const
197193
tag == Js::BackwardPhase &&
198194
!IsPrePass() &&
199195
globOpt->DoLossyIntTypeSpec() &&
200-
!func->IsJitInDebugMode() &&
201-
func->DoGlobOptsForGeneratorFunc();
196+
!func->IsJitInDebugMode();
202197
}
203198

204199
bool
@@ -2565,6 +2560,30 @@ BackwardPass::NeedBailOutOnImplicitCallsForTypedArrayStore(IR::Instr* instr)
25652560
return false;
25662561
}
25672562

2563+
IR::Instr*
2564+
BackwardPass::ProcessPendingPreOpBailOutInfoForYield(IR::Instr* const currentInstr)
2565+
{
2566+
Assert(currentInstr->m_opcode == Js::OpCode::Yield);
2567+
IR::GeneratorBailInInstr* bailInInstr = currentInstr->m_next->m_next->AsGeneratorBailInInstr();
2568+
2569+
BailOutInfo* bailOutInfo = currentInstr->GetBailOutInfo();
2570+
2571+
// Make a copy of all detected constant values before we actually process
2572+
// the bailout info since we will then remove any values that don't need
2573+
// to be restored for the normal bailout cases. As for yields, we still
2574+
// need them for our bailin code.
2575+
bailInInstr->SetConstantValues(bailOutInfo->capturedValues->constantValues);
2576+
2577+
IR::Instr* ret = this->ProcessPendingPreOpBailOutInfo(currentInstr);
2578+
2579+
// We will need list of symbols that have been copy-prop'd to map the correct
2580+
// symbols to restore during bail-in. Since this list is cleared during
2581+
// FillBailOutRecord, make a copy of it now.
2582+
bailInInstr->SetCopyPropSyms(bailOutInfo->usedCapturedValues->copyPropSyms);
2583+
2584+
return ret;
2585+
}
2586+
25682587
IR::Instr*
25692588
BackwardPass::ProcessPendingPreOpBailOutInfo(IR::Instr *const currentInstr)
25702589
{
@@ -2993,6 +3012,11 @@ BackwardPass::ProcessBlock(BasicBlock * block)
29933012
this->currentInstr = instr;
29943013
this->currentRegion = this->currentBlock->GetFirstInstr()->AsLabelInstr()->GetRegion();
29953014

3015+
if (instr->m_opcode == Js::OpCode::Yield && !this->IsCollectionPass())
3016+
{
3017+
this->DisallowMarkTempAcrossYield(this->currentBlock->byteCodeUpwardExposedUsed);
3018+
}
3019+
29963020
IR::Instr * insertedInstr = TryChangeInstrForStackArgOpt();
29973021
if (insertedInstr != nullptr)
29983022
{
@@ -3855,7 +3879,24 @@ BackwardPass::ProcessBlock(BasicBlock * block)
38553879
}
38563880
}
38573881
#endif
3858-
instrPrev = ProcessPendingPreOpBailOutInfo(instr);
3882+
3883+
// Make a copy of upwardExposedUses for our bail-in code, note that we have
3884+
// to do it at the bail-in instruction (right after yield) and not at the yield point
3885+
// since the yield instruction might use some symbols as operands that we don't need when
3886+
// bail-in
3887+
if (instr->IsGeneratorBailInInstr() && this->currentBlock->upwardExposedUses)
3888+
{
3889+
instr->AsGeneratorBailInInstr()->SetUpwardExposedUses(*this->currentBlock->upwardExposedUses);
3890+
}
3891+
3892+
if (instr->m_opcode == Js::OpCode::Yield)
3893+
{
3894+
instrPrev = ProcessPendingPreOpBailOutInfoForYield(instr);
3895+
}
3896+
else
3897+
{
3898+
instrPrev = ProcessPendingPreOpBailOutInfo(instr);
3899+
}
38593900

38603901
#if DBG_DUMP
38613902
TraceInstrUses(block, instr, false);
@@ -6313,6 +6354,27 @@ BackwardPass::ProcessPropertySymUse(PropertySym *propertySym)
63136354
return isLive;
63146355
}
63156356

6357+
void
6358+
BackwardPass::DisallowMarkTempAcrossYield(BVSparse<JitArenaAllocator>* bytecodeUpwardExposed)
6359+
{
6360+
Assert(!this->IsCollectionPass());
6361+
BasicBlock* block = this->currentBlock;
6362+
if (this->DoMarkTempNumbers())
6363+
{
6364+
block->tempNumberTracker->DisallowMarkTempAcrossYield(bytecodeUpwardExposed);
6365+
}
6366+
if (this->DoMarkTempObjects())
6367+
{
6368+
block->tempObjectTracker->DisallowMarkTempAcrossYield(bytecodeUpwardExposed);
6369+
}
6370+
#if DBG
6371+
if (this->DoMarkTempObjectVerify())
6372+
{
6373+
block->tempObjectVerifyTracker->DisallowMarkTempAcrossYield(bytecodeUpwardExposed);
6374+
}
6375+
#endif
6376+
}
6377+
63166378
void
63176379
BackwardPass::MarkTemp(StackSym * sym)
63186380
{

‎lib/Backend/BackwardPass.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ class BackwardPass
4545
bool ProcessByteCodeUsesInstr(IR::Instr * instr);
4646
bool ProcessBailOutInfo(IR::Instr * instr);
4747
void ProcessBailOutInfo(IR::Instr * instr, BailOutInfo * bailOutInfo);
48+
IR::Instr* ProcessPendingPreOpBailOutInfoForYield(IR::Instr* const currentInstr);
4849
IR::Instr* ProcessPendingPreOpBailOutInfo(IR::Instr *const currentInstr);
4950
void ClearDstUseForPostOpLazyBailOut(IR::Instr *instr);
5051
void ProcessBailOutArgObj(BailOutInfo * bailOutInfo, BVSparse<JitArenaAllocator> * byteCodeUpwardExposedUsed);
@@ -56,6 +57,7 @@ class BackwardPass
5657
void ProcessPropertySymOpndUse(IR::PropertySymOpnd *opnd);
5758
bool ProcessPropertySymUse(PropertySym *propertySym);
5859
void ProcessNewScObject(IR::Instr* instr);
60+
void DisallowMarkTempAcrossYield(BVSparse<JitArenaAllocator>* bytecodeUpwardExposed);
5961
void MarkTemp(StackSym * sym);
6062
bool ProcessInlineeStart(IR::Instr* instr);
6163
void ProcessInlineeEnd(IR::Instr* instr);

‎lib/Backend/FlowGraph.cpp

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ FlowGraph::Build(void)
196196
BasicBlock * currBlock = nullptr;
197197
BasicBlock * nextBlock = nullptr;
198198
bool hasCall = false;
199+
bool hasYield = false;
199200

200201
FOREACH_INSTR_IN_FUNC_BACKWARD_EDITING(instr, instrPrev, func)
201202
{
@@ -208,7 +209,9 @@ FlowGraph::Build(void)
208209
nextBlock = currBlock;
209210
currBlock = this->AddBlock(instr->m_next, currLastInstr, nextBlock);
210211
currBlock->hasCall = hasCall;
212+
currBlock->hasYield = hasYield;
211213
hasCall = false;
214+
hasYield = false;
212215
}
213216

214217
currLastInstr = instr;
@@ -243,7 +246,9 @@ FlowGraph::Build(void)
243246
nextBlock = currBlock;
244247
currBlock = this->AddBlock(instr, currLastInstr, nextBlock);
245248
currBlock->hasCall = hasCall;
249+
currBlock->hasYield = hasYield;
246250
hasCall = false;
251+
hasYield = false;
247252
currLastInstr = nullptr;
248253
}
249254

@@ -350,6 +355,11 @@ FlowGraph::Build(void)
350355
break;
351356
}
352357

358+
if (instr->m_opcode == Js::OpCode::Yield)
359+
{
360+
hasYield = true;
361+
}
362+
353363
if (OpCodeAttr::UseAllFields(instr->m_opcode))
354364
{
355365
// UseAllFields opcode are call instruction or opcode that would call.
@@ -1400,6 +1410,10 @@ FlowGraph::WalkLoopBlocks(BasicBlock *block, Loop *loop, JitArenaAllocator *temp
14001410
{
14011411
loop->SetHasCall();
14021412
}
1413+
if (pred->loop->hasYield)
1414+
{
1415+
loop->SetHasYield();
1416+
}
14031417
loop->SetImplicitCallFlags(pred->loop->GetImplicitCallFlags());
14041418
}
14051419
// Add pred to loop bit vector
@@ -1430,6 +1444,10 @@ FlowGraph::AddBlockToLoop(BasicBlock *block, Loop *loop)
14301444
{
14311445
loop->SetHasCall();
14321446
}
1447+
if (block->hasYield)
1448+
{
1449+
loop->SetHasYield();
1450+
}
14331451
}
14341452

14351453
///----------------------------------------------------------------------------
@@ -3500,6 +3518,29 @@ Loop::SetHasCall()
35003518
while (current != nullptr);
35013519
}
35023520

3521+
void
3522+
Loop::SetHasYield()
3523+
{
3524+
Loop* current = this;
3525+
do
3526+
{
3527+
if (current->hasYield)
3528+
{
3529+
#if DBG
3530+
current = current->parent;
3531+
while (current)
3532+
{
3533+
Assert(current->hasYield);
3534+
current = current->parent;
3535+
}
3536+
#endif
3537+
break;
3538+
}
3539+
current->hasYield = true;
3540+
current = current->parent;
3541+
} while (current != nullptr);
3542+
}
3543+
35033544
void
35043545
Loop::SetImplicitCallFlags(Js::ImplicitCallFlags newFlags)
35053546
{
@@ -3570,7 +3611,7 @@ Loop::CanHoistInvariants() const
35703611
return false;
35713612
}
35723613

3573-
return true;
3614+
return !this->hasYield;
35743615
}
35753616

35763617
IR::LabelInstr *

‎lib/Backend/FlowGraph.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,7 @@ class BasicBlock
373373
uint8 isDead:1;
374374
uint8 isLoopHeader:1;
375375
uint8 hasCall:1;
376+
uint8 hasYield:1;
376377
uint8 isVisited:1;
377378
uint8 isAirLockCompensationBlock:1;
378379
uint8 beginsBailOnNoProfile:1;
@@ -431,6 +432,7 @@ class BasicBlock
431432
isDead(false),
432433
isLoopHeader(false),
433434
hasCall(false),
435+
hasYield(false),
434436
liveFixedFields(nullptr),
435437
upwardExposedUses(nullptr),
436438
upwardExposedFields(nullptr),
@@ -616,6 +618,7 @@ class Loop
616618
bool hasDeadStoreCollectionPass : 1;
617619
bool hasDeadStorePrepass : 1;
618620
bool hasCall : 1;
621+
bool hasYield : 1;
619622
bool hasHoistedFields : 1;
620623
bool needImplicitCallBailoutChecksForJsArrayCheckHoist : 1;
621624
bool allFieldsKilled : 1;
@@ -765,6 +768,7 @@ class Loop
765768
bool CanHoistInvariants() const;
766769
bool CanDoFieldCopyProp();
767770
void SetHasCall();
771+
void SetHasYield();
768772
IR::LabelInstr * GetLoopTopInstr() const;
769773
void SetLoopTopInstr(IR::LabelInstr * loopTop);
770774
Func * GetFunc() const { return GetLoopTopInstr()->m_func; }

‎lib/Backend/Func.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
139139
, vtableMap(nullptr)
140140
#endif
141141
, m_yieldOffsetResumeLabelList(nullptr)
142-
, m_bailOutNoSaveLabel(nullptr)
142+
, m_bailOutForElidedYieldInsertionPoint(nullptr)
143143
, constantAddressRegOpnd(alloc)
144144
, lastConstantAddressRegLoadInstr(nullptr)
145145
, m_totalJumpTableSizeInBytesForSwitchStatements(0)
@@ -867,13 +867,6 @@ Func::AjustLocalVarSlotOffset()
867867
}
868868
#endif
869869

870-
bool
871-
Func::DoGlobOptsForGeneratorFunc() const
872-
{
873-
// Disable GlobOpt optimizations for generators initially. Will visit and enable each one by one.
874-
return !GetJITFunctionBody()->IsCoroutine();
875-
}
876-
877870
bool
878871
Func::DoSimpleJitDynamicProfile() const
879872
{

‎lib/Backend/Func.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ class Func
206206
!PHASE_OFF(Js::GlobOptPhase, this) && !IsSimpleJit() &&
207207
(!GetTopFunc()->HasTry() || GetTopFunc()->CanOptimizeTryCatch()) &&
208208
(!GetTopFunc()->HasFinally() || GetTopFunc()->CanOptimizeTryFinally()) &&
209-
!GetTopFunc()->GetJITFunctionBody()->IsCoroutine();
209+
(!GetTopFunc()->GetJITFunctionBody()->IsCoroutine() || !PHASE_OFF(Js::GeneratorGlobOptPhase, this));
210210
}
211211

212212
bool DoInline() const
@@ -329,8 +329,6 @@ class Func
329329
void AjustLocalVarSlotOffset();
330330
#endif
331331

332-
bool DoGlobOptsForGeneratorFunc() const;
333-
334332
static int32 AdjustOffsetValue(int32 offset);
335333

336334
static inline uint32 GetDiagLocalSlotSize()
@@ -998,7 +996,7 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
998996

999997
uint32 m_inlineeId;
1000998

1001-
IR::LabelInstr * m_bailOutNoSaveLabel;
999+
IR::Instr * m_bailOutForElidedYieldInsertionPoint;
10021000

10031001
private:
10041002
Js::EntryPointInfo* m_entryPointInfo; // for in-proc JIT only

0 commit comments

Comments
 (0)