Skip to content

Commit c7ca1c2

Browse files
rhuanjlrhuanjl
committed
Generator/Async JIT Fix nested For-In loops
- For in loops use an enumerator object - for optimisation reasons this is cached in a known location - in nested for-in loops containing yield cache could fail to restore - remove a step from the process and bring load nearer to usage
1 parent 27a0c16 commit c7ca1c2

5 files changed

Lines changed: 39 additions & 73 deletions

File tree

‎lib/Backend/Func.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56
#include "Backend.h"
@@ -149,7 +150,7 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
149150
, m_forInEnumeratorArrayOffset(-1)
150151
, argInsCount(0)
151152
, m_globalObjTypeSpecFldInfoArray(nullptr)
152-
, m_forInEnumeratorForGeneratorSym(nullptr)
153+
, m_generatorFrameSym(nullptr)
153154
#if LOWER_SPLIT_INT64
154155
, m_int64SymPairMap(nullptr)
155156
#endif

‎lib/Backend/Func.h‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56
#pragma once
@@ -1063,21 +1064,21 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
10631064
StackSym* m_loopParamSym;
10641065
StackSym* m_bailoutReturnValueSym;
10651066
StackSym* m_hasBailedOutSym;
1066-
StackSym* m_forInEnumeratorForGeneratorSym;
1067+
StackSym* m_generatorFrameSym;
10671068

10681069
public:
10691070
StackSym* tempSymDouble;
10701071
StackSym* tempSymBool;
10711072

1072-
void SetForInEnumeratorSymForGeneratorSym(StackSym* sym)
1073+
void SetGeneratorFrameSym(StackSym* sym)
10731074
{
1074-
Assert(this->m_forInEnumeratorForGeneratorSym == nullptr);
1075-
this->m_forInEnumeratorForGeneratorSym = sym;
1075+
Assert(this->m_generatorFrameSym == nullptr);
1076+
this->m_generatorFrameSym = sym;
10761077
}
10771078

1078-
StackSym* GetForInEnumeratorSymForGeneratorSym() const
1079+
StackSym* GetGeneratorFrameSym() const
10791080
{
1080-
return this->m_forInEnumeratorForGeneratorSym;
1081+
return this->m_generatorFrameSym;
10811082
}
10821083

10831084
// StackSyms' corresponding getters/setters

‎lib/Backend/IRBuilder.cpp‎

Lines changed: 25 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56
#include "Backend.h"
@@ -1255,7 +1256,7 @@ IRBuilder::EnsureLoopBodyForInEnumeratorArrayOpnd()
12551256
}
12561257

12571258
IR::Opnd *
1258-
IRBuilder::BuildForInEnumeratorOpnd(uint forInLoopLevel)
1259+
IRBuilder::BuildForInEnumeratorOpnd(uint forInLoopLevel, uint32 offset)
12591260
{
12601261
Assert(forInLoopLevel < this->m_func->GetJITFunctionBody()->GetForInLoopDepth());
12611262
if (this->IsLoopBody())
@@ -1270,7 +1271,7 @@ IRBuilder::BuildForInEnumeratorOpnd(uint forInLoopLevel)
12701271
else if (this->m_func->GetJITFunctionBody()->IsCoroutine())
12711272
{
12721273
return IR::IndirOpnd::New(
1273-
this->m_generatorJumpTable.EnsureForInEnumeratorArrayOpnd(),
1274+
this->m_generatorJumpTable.BuildForInEnumeratorArrayOpnd(offset),
12741275
forInLoopLevel * sizeof(Js::ForInObjectEnumerator),
12751276
TyMachPtr,
12761277
this->m_func
@@ -2949,7 +2950,7 @@ IRBuilder::BuildProfiledReg1Unsigned1(Js::OpCode newOpcode, uint32 offset, Js::R
29492950
if (newOpcode == Js::OpCode::InitForInEnumerator)
29502951
{
29512952
IR::RegOpnd * src1Opnd = this->BuildSrcOpnd(R0);
2952-
IR::Opnd * src2Opnd = this->BuildForInEnumeratorOpnd(C1);
2953+
IR::Opnd * src2Opnd = this->BuildForInEnumeratorOpnd(C1, offset);
29532954
IR::Instr *instr = IR::ProfiledInstr::New(Js::OpCode::InitForInEnumerator, nullptr, src1Opnd, src2Opnd, m_func);
29542955
instr->AsProfiledInstr()->u.profileId = profileId;
29552956
this->AddInstr(instr, offset);
@@ -3084,7 +3085,7 @@ IRBuilder::BuildReg1Unsigned1(Js::OpCode newOpcode, uint offset, Js::RegSlot R0,
30843085
{
30853086
IR::Instr *instr = IR::Instr::New(Js::OpCode::InitForInEnumerator, m_func);
30863087
instr->SetSrc1(this->BuildSrcOpnd(R0));
3087-
instr->SetSrc2(this->BuildForInEnumeratorOpnd(C1));
3088+
instr->SetSrc2(this->BuildForInEnumeratorOpnd(C1, offset));
30883089
this->AddInstr(instr, offset);
30893090
return;
30903091
}
@@ -6935,7 +6936,7 @@ IRBuilder::BuildBrReg1Unsigned1(Js::OpCode newOpcode, uint32 offset)
69356936
void
69366937
IRBuilder::BuildBrBReturn(Js::OpCode newOpcode, uint32 offset, Js::RegSlot DestRegSlot, uint32 forInLoopLevel, uint32 targetOffset)
69376938
{
6938-
IR::Opnd *srcOpnd = this->BuildForInEnumeratorOpnd(forInLoopLevel);
6939+
IR::Opnd *srcOpnd = this->BuildForInEnumeratorOpnd(forInLoopLevel, offset);
69396940
IR::RegOpnd * destOpnd = this->BuildDstOpnd(DestRegSlot);
69406941
IR::BranchInstr * branchInstr = IR::BranchInstr::New(newOpcode, destOpnd, nullptr, srcOpnd, m_func);
69416942
this->AddBranchInstr(branchInstr, offset, targetOffset);
@@ -7922,14 +7923,12 @@ IRBuilder::GeneratorJumpTable::BuildJumpTable()
79227923
//
79237924
// s1 = Ld_A prm1
79247925
// s2 = Ld_A s1[offset of JavascriptGenerator::frame]
7925-
// BrNotAddr_A s2 !nullptr $initializationCode
7926+
// BrNotAddr_A s2 !nullptr $jumpTable
79267927
//
79277928
// $createInterpreterStackFrame:
79287929
// call helper
79297930
//
7930-
// $initializationCode:
7931-
// load for-in enumerator address from interpreter stack frame
7932-
//
7931+
// Br $startOfFunc
79337932
//
79347933
// $jumpTable:
79357934
//
@@ -7963,23 +7962,25 @@ IRBuilder::GeneratorJumpTable::BuildJumpTable()
79637962
IR::LabelInstr* functionBegin = IR::LabelInstr::New(Js::OpCode::Label, this->m_func);
79647963
LABELNAMESET(functionBegin, "GeneratorFunctionBegin");
79657964

7966-
IR::LabelInstr* initCode = IR::LabelInstr::New(Js::OpCode::Label, this->m_func);
7967-
LABELNAMESET(initCode, "GeneratorInitializationAndJumpTable");
7965+
IR::LabelInstr* jumpTable = IR::LabelInstr::New(Js::OpCode::Label, this->m_func);
7966+
LABELNAMESET(jumpTable, "GeneratorJumpTable");
79687967

7969-
// BrNotAddr_A s2 nullptr $initializationCode
7970-
IR::BranchInstr* skipCreateInterpreterFrame = IR::BranchInstr::New(Js::OpCode::BrNotAddr_A, initCode, genFrameOpnd, IR::AddrOpnd::NewNull(this->m_func), this->m_func);
7968+
// If there is already a stack frame, generator function has previously begun execution - don't recreate, skip down to jump table
7969+
// BrNotAddr_A s2 nullptr $jumpTable
7970+
IR::BranchInstr* skipCreateInterpreterFrame = IR::BranchInstr::New(Js::OpCode::BrNotAddr_A, jumpTable, genFrameOpnd, IR::AddrOpnd::NewNull(this->m_func), this->m_func);
79717971
this->m_irBuilder->AddInstr(skipCreateInterpreterFrame, this->m_irBuilder->m_functionStartOffset);
79727972

79737973
// Create interpreter stack frame
79747974
IR::Instr* createInterpreterFrame = IR::Instr::New(Js::OpCode::GeneratorCreateInterpreterStackFrame, genFrameOpnd /* dst */, genRegOpnd /* src */, this->m_func);
79757975
this->m_irBuilder->AddInstr(createInterpreterFrame, this->m_irBuilder->m_functionStartOffset);
79767976

7977+
// Having created the frame, skip over the jump table and start executing from the beginning of the function
79777978
IR::BranchInstr* skipJumpTable = IR::BranchInstr::New(Js::OpCode::Br, functionBegin, this->m_func);
79787979
this->m_irBuilder->AddInstr(skipJumpTable, this->m_irBuilder->m_functionStartOffset);
79797980

7980-
// Label to insert any initialization code
7981-
// $initializationCode:
7982-
this->m_irBuilder->AddInstr(initCode, this->m_irBuilder->m_functionStartOffset);
7981+
// Label for start of jumpTable - where we look for the correct Yield resume point
7982+
// $jumpTable:
7983+
this->m_irBuilder->AddInstr(jumpTable, this->m_irBuilder->m_functionStartOffset);
79837984

79847985
// s3 = Ld_A s2[offset of InterpreterStackFrame::m_reader.m_currentLocation]
79857986
IR::RegOpnd* curLocOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func);
@@ -8015,46 +8016,24 @@ IRBuilder::GeneratorJumpTable::BuildJumpTable()
80158016

80168017
this->m_irBuilder->AddInstr(functionBegin, this->m_irBuilder->m_functionStartOffset);
80178018

8018-
// Save these values for later use
8019-
this->m_initLabel = initCode;
8019+
// Save this value for later use
80208020
this->m_generatorFrameOpnd = genFrameOpnd;
8021+
this->m_func->SetGeneratorFrameSym(genFrameOpnd->GetStackSym());
80218022

80228023
return this->m_irBuilder->m_lastInstr;
80238024
}
80248025

8025-
IR::LabelInstr*
8026-
IRBuilder::GeneratorJumpTable::GetInitLabel() const
8027-
{
8028-
Assert(this->m_initLabel != nullptr);
8029-
return this->m_initLabel;
8030-
}
8031-
80328026
IR::RegOpnd*
8033-
IRBuilder::GeneratorJumpTable::CreateForInEnumeratorArrayOpnd()
8034-
{
8035-
Assert(this->m_initLabel != nullptr);
8027+
IRBuilder::GeneratorJumpTable::BuildForInEnumeratorArrayOpnd(uint32 offset)
8028+
{
80368029
Assert(this->m_generatorFrameOpnd != nullptr);
80378030

8038-
IR::RegOpnd* forInEnumeratorOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func);
8039-
IR::Instr* instr = IR::Instr::New(
8040-
Js::OpCode::Ld_A,
8041-
forInEnumeratorOpnd,
8031+
IR::RegOpnd* forInEnumeratorArrayOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func);
8032+
IR::Instr* instr = IR::Instr::New(Js::OpCode::Ld_A, forInEnumeratorArrayOpnd,
80428033
POINTER_OFFSET(this->m_generatorFrameOpnd, Js::InterpreterStackFrame::GetOffsetOfForInEnumerators(), ForInEnumerators),
80438034
this->m_func
80448035
);
8045-
this->m_initLabel->InsertAfter(instr);
8046-
8047-
return forInEnumeratorOpnd;
8048-
}
8049-
8050-
IR::RegOpnd*
8051-
IRBuilder::GeneratorJumpTable::EnsureForInEnumeratorArrayOpnd()
8052-
{
8053-
if (this->m_forInEnumeratorArrayOpnd == nullptr)
8054-
{
8055-
this->m_forInEnumeratorArrayOpnd = this->CreateForInEnumeratorArrayOpnd();
8056-
this->m_func->SetForInEnumeratorSymForGeneratorSym(m_forInEnumeratorArrayOpnd->GetStackSym());
8057-
}
8036+
this->m_irBuilder->AddInstr(instr, offset);
80588037

8059-
return this->m_forInEnumeratorArrayOpnd;
8038+
return forInEnumeratorArrayOpnd;
80608039
}

‎lib/Backend/IRBuilder.h‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56
#pragma once
@@ -227,7 +228,7 @@ class IRBuilder
227228
IR::RegOpnd * BuildSrcOpnd(Js::RegSlot srcRegSlot, IRType type = TyVar);
228229
IR::AddrOpnd * BuildAuxArrayOpnd(AuxArrayValue auxArrayType, uint32 auxArrayOffset);
229230
IR::Opnd * BuildAuxObjectLiteralTypeRefOpnd(int objectId);
230-
IR::Opnd * BuildForInEnumeratorOpnd(uint forInLoopLevel);
231+
IR::Opnd * BuildForInEnumeratorOpnd(uint forInLoopLevel, uint32 offset);
231232
IR::RegOpnd * EnsureLoopBodyForInEnumeratorArrayOpnd();
232233
private:
233234
uint AddStatementBoundary(uint statementIndex, uint offset);
@@ -366,29 +367,12 @@ class IRBuilder
366367
Func* const m_func;
367368
IRBuilder* const m_irBuilder;
368369

369-
// for-in enumerators are allocated on the heap for jit'd loop body
370-
// and on the stack for all other cases (the interpreter frame will
371-
// reuses them when bailing out). But because we have the concept of
372-
// "bailing in" for generator, reusing enumerators allocated on the stack
373-
// would not work. So we have to allocate them on the generator's interpreter
374-
// frame instead. This operand is loaded as part of the jump table, before we
375-
// jump to any of the resume point.
376-
IR::RegOpnd* m_forInEnumeratorArrayOpnd = nullptr;
377-
378370
IR::RegOpnd* m_generatorFrameOpnd = nullptr;
379371

380-
// This label is used to insert any initialization code that might be needed
381-
// when bailing in and before jumping to any of the resume points.
382-
// As of now, we only need to load the operand for for-in enumerator.
383-
IR::LabelInstr* m_initLabel = nullptr;
384-
385-
IR::RegOpnd* CreateForInEnumeratorArrayOpnd();
386-
387372
public:
388373
GeneratorJumpTable(Func* func, IRBuilder* irBuilder);
389374
IR::Instr* BuildJumpTable();
390-
IR::LabelInstr* GetInitLabel() const;
391-
IR::RegOpnd* EnsureForInEnumeratorArrayOpnd();
375+
IR::RegOpnd* BuildForInEnumeratorArrayOpnd(uint32 offset);
392376
};
393377

394378
GeneratorJumpTable m_generatorJumpTable;

‎lib/Backend/LinearScan.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft Corporation and contributors. All rights reserved.
3+
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56

@@ -5301,7 +5302,7 @@ bool LinearScan::GeneratorBailIn::NeedsReloadingBackendSymWhenBailingIn(StackSym
53015302
{
53025303
// for-in enumerator in generator is loaded as part of the resume jump table.
53035304
// By the same reasoning as `initializedRegs`'s, we don't have to restore this whether or not it's been spilled.
5304-
if (this->func->GetForInEnumeratorSymForGeneratorSym() && this->func->GetForInEnumeratorSymForGeneratorSym()->m_id == sym->m_id)
5305+
if (this->func->GetGeneratorFrameSym() && this->func->GetGeneratorFrameSym()->m_id == sym->m_id)
53055306
{
53065307
return false;
53075308
}

0 commit comments

Comments
 (0)