Skip to content

Commit 55ac99f

Browse files
nhat-nguyennhat-nguyen
committed
Enable globopt for generator functions
1 parent d101e8e commit 55ac99f

15 files changed

+420
-186
lines changed

‎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/Func.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
138138
, vtableMap(nullptr)
139139
#endif
140140
, m_yieldOffsetResumeLabelList(nullptr)
141-
, m_bailOutNoSaveLabel(nullptr)
141+
, m_bailOutForElidedYieldInsertionPoint(nullptr)
142142
, constantAddressRegOpnd(alloc)
143143
, lastConstantAddressRegLoadInstr(nullptr)
144144
, m_totalJumpTableSizeInBytesForSwitchStatements(0)
@@ -866,13 +866,6 @@ Func::AjustLocalVarSlotOffset()
866866
}
867867
#endif
868868

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

‎lib/Backend/Func.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -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()
@@ -997,7 +995,7 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
997995

998996
uint32 m_inlineeId;
999997

1000-
IR::LabelInstr * m_bailOutNoSaveLabel;
998+
IR::Instr * m_bailOutForElidedYieldInsertionPoint;
1001999

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

‎lib/Backend/GlobHashTable.h

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,30 @@ class Key
3131
static uint Get(ExprHash hash) { return static_cast<uint>(hash); }
3232
};
3333

34-
#define FOREACH_GLOBHASHTABLE_ENTRY(bucket, hashTable) \
34+
#define FOREACH_VALUEHASHTABLE_ENTRY(BucketType, bucket, hashTable) \
3535
for (uint _iterHash = 0; _iterHash < (hashTable)->tableSize; _iterHash++) \
3636
{ \
37-
FOREACH_SLISTBASE_ENTRY(GlobHashBucket, bucket, &(hashTable)->table[_iterHash]) \
37+
FOREACH_SLISTBASE_ENTRY(BucketType, bucket, &(hashTable)->table[_iterHash]) \
3838
{
3939

4040

41-
#define NEXT_GLOBHASHTABLE_ENTRY \
41+
#define NEXT_VALUEHASHTABLE_ENTRY \
4242
} \
4343
NEXT_SLISTBASE_ENTRY; \
4444
}
4545

46+
#define FOREACH_VALUEHASHTABLE_ENTRY_EDITING(BucketType, bucket, hashTable, iter) \
47+
for (uint _iterHash = 0; _iterHash < (hashTable)->tableSize; _iterHash++) \
48+
{ \
49+
FOREACH_SLISTBASE_ENTRY_EDITING(BucketType, bucket, &(hashTable)->table[_iterHash], iter) \
50+
{
51+
52+
53+
#define NEXT_VALUEHASHTABLE_ENTRY_EDITING \
54+
} \
55+
NEXT_SLISTBASE_ENTRY_EDITING; \
56+
}
57+
4658
template<typename TData, typename TElement>
4759
class ValueHashTable
4860
{
@@ -390,28 +402,28 @@ class ValueHashTable
390402
#if DBG_DUMP
391403
void Dump()
392404
{
393-
FOREACH_GLOBHASHTABLE_ENTRY(bucket, this)
405+
FOREACH_VALUEHASHTABLE_ENTRY(HashBucket, bucket, this)
394406
{
395407

396408
Output::Print(_u("%4d => "), bucket.value);
397409
bucket.element->Dump();
398410
Output::Print(_u("\n"));
399411
Output::Print(_u("\n"));
400412
}
401-
NEXT_GLOBHASHTABLE_ENTRY;
413+
NEXT_VALUEHASHTABLE_ENTRY;
402414
}
403415

404416
void Dump(void (*valueDump)(TData))
405417
{
406418
Output::Print(_u("\n-------------------------------------------------------------------------------------------------\n"));
407-
FOREACH_GLOBHASHTABLE_ENTRY(bucket, this)
419+
FOREACH_VALUEHASHTABLE_ENTRY(HashBucket, bucket, this)
408420
{
409421
valueDump(bucket.value);
410422
Output::Print(_u(" => "), bucket.value);
411423
bucket.element->Dump();
412424
Output::Print(_u("\n"));
413425
}
414-
NEXT_GLOBHASHTABLE_ENTRY;
426+
NEXT_VALUEHASHTABLE_ENTRY;
415427
}
416428
#endif
417429

‎lib/Backend/GlobOpt.cpp

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -838,14 +838,19 @@ GlobOpt::TryTailDup(IR::BranchInstr *tailBranch)
838838
}
839839

840840
void
841-
GlobOpt::ToVar(BVSparse<JitArenaAllocator> *bv, BasicBlock *block)
841+
GlobOpt::ToVar(BVSparse<JitArenaAllocator> *bv, BasicBlock *block, IR::Instr* insertBeforeInstr /* = nullptr */)
842842
{
843843
FOREACH_BITSET_IN_SPARSEBV(id, bv)
844844
{
845845
StackSym *stackSym = this->func->m_symTable->FindStackSym(id);
846846
IR::RegOpnd *newOpnd = IR::RegOpnd::New(stackSym, TyVar, this->func);
847-
IR::Instr *lastInstr = block->GetLastInstr();
848-
if (lastInstr->IsBranchInstr() || lastInstr->m_opcode == Js::OpCode::BailTarget)
847+
IR::Instr* lastInstr = block->GetLastInstr();
848+
849+
if (insertBeforeInstr != nullptr)
850+
{
851+
this->ToVar(insertBeforeInstr, newOpnd, block, nullptr, false);
852+
}
853+
else if (lastInstr->IsBranchInstr() || lastInstr->m_opcode == Js::OpCode::BailTarget)
849854
{
850855
// If branch is using this symbol, hoist the operand as the ToVar load will get
851856
// inserted right before the branch.
@@ -2427,15 +2432,15 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
24272432
return instrNext;
24282433
}
24292434

2430-
if (!instr->IsRealInstr() || instr->IsByteCodeUsesInstr() || instr->m_opcode == Js::OpCode::Conv_Bool)
2435+
if (instr->m_opcode == Js::OpCode::Yield)
24312436
{
2432-
return instrNext;
2437+
// TODO[generators][ianhall]: Can this and the FillBailOutInfo call below be moved to after Src1 and Src2 so that Yield can be optimized right up to the actual yield?
2438+
this->ProcessKills(instr);
24332439
}
24342440

2435-
if (instr->m_opcode == Js::OpCode::Yield)
2441+
if (!instr->IsRealInstr() || instr->IsByteCodeUsesInstr() || instr->m_opcode == Js::OpCode::Conv_Bool)
24362442
{
2437-
// TODO[generators][ianhall]: Can this and the FillBailOutInfo call below be moved to after Src1 and Src2 so that Yield can be optimized right up to the actual yield?
2438-
CurrentBlockData()->KillStateForGeneratorYield();
2443+
return instrNext;
24392444
}
24402445

24412446
if (!IsLoopPrePass())
@@ -3033,13 +3038,13 @@ GlobOpt::OptDst(
30333038
else if (dstVal)
30343039
{
30353040
opnd->SetValueType(dstVal->GetValueInfo()->Type());
3036-
3041+
#if 0
30373042
if(currentBlock->loop &&
30383043
!IsLoopPrePass() &&
30393044
(instr->m_opcode == Js::OpCode::Ld_A || instr->m_opcode == Js::OpCode::Ld_I4) &&
30403045
instr->GetSrc1()->IsRegOpnd() &&
30413046
!func->IsJitInDebugMode() &&
3042-
func->DoGlobOptsForGeneratorFunc())
3047+
this->GetJITFunctionBody()->IsCoroutine())
30433048
{
30443049
// Look for the following patterns:
30453050
//
@@ -3103,6 +3108,7 @@ GlobOpt::OptDst(
31033108
this->SetSymStoreDirect(dstVal->GetValueInfo(), dstVarSym);
31043109
} while(false);
31053110
}
3111+
#endif
31063112
}
31073113

31083114
this->ValueNumberObjectType(opnd, instr);
@@ -3658,15 +3664,6 @@ GlobOpt::CopyProp(IR::Opnd *opnd, IR::Instr *instr, Value *val, IR::IndirOpnd *p
36583664
return opnd;
36593665
}
36603666

3661-
if (!this->func->DoGlobOptsForGeneratorFunc())
3662-
{
3663-
// Don't copy prop in generator functions because non-bytecode temps that span a yield
3664-
// cannot be saved and restored by the current bail-out mechanics utilized by generator
3665-
// yield/resume.
3666-
// TODO[generators][ianhall]: Enable copy-prop at least for in between yields.
3667-
return opnd;
3668-
}
3669-
36703667
if (instr->m_opcode == Js::OpCode::CheckFixedFld || instr->m_opcode == Js::OpCode::CheckPropertyGuardAndLoadType)
36713668
{
36723669
// Don't copy prop into CheckFixedFld or CheckPropertyGuardAndLoadType
@@ -14555,6 +14552,10 @@ GlobOpt::PreLowerCanonicalize(IR::Instr *instr, Value **pSrc1Val, Value **pSrc2V
1455514552
void
1455614553
GlobOpt::ProcessKills(IR::Instr *instr)
1455714554
{
14555+
if (instr->m_opcode == Js::OpCode::Yield)
14556+
{
14557+
this->CurrentBlockData()->KillStateForGeneratorYield(instr);
14558+
}
1455814559
this->ProcessFieldKills(instr);
1455914560
this->ProcessValueKills(instr);
1456014561
this->ProcessArrayValueKills(instr);
@@ -15695,7 +15696,7 @@ GlobOpt::DoConstFold() const
1569515696
bool
1569615697
GlobOpt::IsTypeSpecPhaseOff(Func const *func)
1569715698
{
15698-
return PHASE_OFF(Js::TypeSpecPhase, func) || func->IsJitInDebugMode() || !func->DoGlobOptsForGeneratorFunc();
15699+
return PHASE_OFF(Js::TypeSpecPhase, func) || func->IsJitInDebugMode();
1569915700
}
1570015701

1570115702
bool
@@ -15802,8 +15803,7 @@ GlobOpt::DoArrayCheckHoist(Func const * const func)
1580215803
return
1580315804
!PHASE_OFF(Js::ArrayCheckHoistPhase, func) &&
1580415805
!func->IsArrayCheckHoistDisabled() &&
15805-
!func->IsJitInDebugMode() && // StElemI fast path is not allowed when in debug mode, so it cannot have bailout
15806-
func->DoGlobOptsForGeneratorFunc();
15806+
!func->IsJitInDebugMode(); // StElemI fast path is not allowed when in debug mode, so it cannot have bailout
1580715807
}
1580815808

1580915809
bool

‎lib/Backend/GlobOpt.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ class GlobOpt
742742
void InsertCloneStrs(BasicBlock *toBlock, GlobOptBlockData *toData, GlobOptBlockData *fromData);
743743
void InsertValueCompensation(BasicBlock *const predecessor, BasicBlock *const successor, const SymToValueInfoMap *symsRequiringCompensationToMergedValueInfoMap);
744744
IR::Instr * ToVarUses(IR::Instr *instr, IR::Opnd *opnd, bool isDst, Value *val);
745-
void ToVar(BVSparse<JitArenaAllocator> *bv, BasicBlock *block);
745+
void ToVar(BVSparse<JitArenaAllocator> *bv, BasicBlock *block, IR::Instr* insertBeforeInstr = nullptr);
746746
IR::Instr * ToVar(IR::Instr *instr, IR::RegOpnd *regOpnd, BasicBlock *block, Value *val, bool needsUpdate);
747747
void ToInt32(BVSparse<JitArenaAllocator> *bv, BasicBlock *block, bool lossy, IR::Instr *insertBeforeInstr = nullptr);
748748
void ToFloat64(BVSparse<JitArenaAllocator> *bv, BasicBlock *block);

0 commit comments

Comments
 (0)