diff --git a/lib/Backend/Func.cpp b/lib/Backend/Func.cpp index 8e25640b935..87e2262fe56 100644 --- a/lib/Backend/Func.cpp +++ b/lib/Backend/Func.cpp @@ -150,7 +150,6 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem, , m_forInEnumeratorArrayOffset(-1) , argInsCount(0) , m_globalObjTypeSpecFldInfoArray(nullptr) - , m_generatorFrameSym(nullptr) #if LOWER_SPLIT_INT64 , m_int64SymPairMap(nullptr) #endif diff --git a/lib/Backend/Func.h b/lib/Backend/Func.h index f1ca1dff944..5d14fa50a05 100644 --- a/lib/Backend/Func.h +++ b/lib/Backend/Func.h @@ -1064,23 +1064,11 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece; StackSym* m_loopParamSym; StackSym* m_bailoutReturnValueSym; StackSym* m_hasBailedOutSym; - StackSym* m_generatorFrameSym; public: StackSym* tempSymDouble; StackSym* tempSymBool; - void SetGeneratorFrameSym(StackSym* sym) - { - Assert(this->m_generatorFrameSym == nullptr); - this->m_generatorFrameSym = sym; - } - - StackSym* GetGeneratorFrameSym() const - { - return this->m_generatorFrameSym; - } - // StackSyms' corresponding getters/setters void SetInlineeFrameStartSym(StackSym* sym) { diff --git a/lib/Backend/IRBuilder.cpp b/lib/Backend/IRBuilder.cpp index 72ded2a0d5c..41c2f938794 100644 --- a/lib/Backend/IRBuilder.cpp +++ b/lib/Backend/IRBuilder.cpp @@ -7954,21 +7954,35 @@ IRBuilder::GeneratorJumpTable::BuildJumpTable() this->m_irBuilder->AddInstr(functionBegin, this->m_irBuilder->m_functionStartOffset); - // Save this value for later use - this->m_generatorFrameOpnd = genFrameOpnd; - this->m_func->SetGeneratorFrameSym(genFrameOpnd->GetStackSym()); - return this->m_irBuilder->m_lastInstr; } IR::RegOpnd* IRBuilder::GeneratorJumpTable::BuildForInEnumeratorArrayOpnd(uint32 offset) { - Assert(this->m_generatorFrameOpnd != nullptr); + // s1 = Ld_A prm1 + StackSym* genParamSym = StackSym::NewParamSlotSym(1, this->m_func); + this->m_func->SetArgOffset(genParamSym, LowererMD::GetFormalParamOffset() * MachPtr); + + IR::SymOpnd* genParamOpnd = IR::SymOpnd::New(genParamSym, TyMachPtr, this->m_func); + IR::RegOpnd* genRegOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func); + IR::Instr* instr = IR::Instr::New(Js::OpCode::Ld_A, genRegOpnd, genParamOpnd, this->m_func); + this->m_irBuilder->AddInstr(instr, offset); + + // s2 = Ld_A s1[offset of JavascriptGenerator::frame] + IR::RegOpnd* genFrameOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func); + instr = IR::Instr::New( + Js::OpCode::Ld_A, + genFrameOpnd, + POINTER_OFFSET(genRegOpnd, Js::JavascriptGenerator::GetFrameOffset(), GeneratorFrame), + this->m_func + ); + this->m_irBuilder->AddInstr(instr, offset); + // s3 = Ld_A s2[offset of ForInEnumerator] IR::RegOpnd* forInEnumeratorArrayOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func); - IR::Instr* instr = IR::Instr::New(Js::OpCode::Ld_A, forInEnumeratorArrayOpnd, - POINTER_OFFSET(this->m_generatorFrameOpnd, Js::InterpreterStackFrame::GetOffsetOfForInEnumerators(), ForInEnumerators), + instr = IR::Instr::New(Js::OpCode::Ld_A, forInEnumeratorArrayOpnd, + POINTER_OFFSET(genFrameOpnd, Js::InterpreterStackFrame::GetOffsetOfForInEnumerators(), ForInEnumerators), this->m_func ); this->m_irBuilder->AddInstr(instr, offset); diff --git a/lib/Backend/IRBuilder.h b/lib/Backend/IRBuilder.h index 1ad9b383be8..498f11d3d88 100644 --- a/lib/Backend/IRBuilder.h +++ b/lib/Backend/IRBuilder.h @@ -366,8 +366,6 @@ class IRBuilder Func* const m_func; IRBuilder* const m_irBuilder; - IR::RegOpnd* m_generatorFrameOpnd = nullptr; - public: GeneratorJumpTable(Func* func, IRBuilder* irBuilder); IR::Instr* BuildJumpTable(); diff --git a/lib/Backend/LinearScan.cpp b/lib/Backend/LinearScan.cpp index aa214b4f477..812ce78a2ea 100644 --- a/lib/Backend/LinearScan.cpp +++ b/lib/Backend/LinearScan.cpp @@ -3339,7 +3339,7 @@ LinearScan::KillImplicitRegs(IR::Instr *instr) return; } - if (instr->m_opcode == Js::OpCode::Yield) + if (instr->m_opcode == Js::OpCode::GeneratorBailInLabel) { this->bailIn.SpillRegsForBailIn(); return; @@ -5041,6 +5041,7 @@ IR::Instr* LinearScan::GeneratorBailIn::GenerateBailIn(IR::GeneratorBailInInstr* // - We don't need to restore argObjSyms because StackArgs is currently not enabled // Commented out here in case we do want to enable it in the future: // this->InsertRestoreSymbols(bailOutInfo->capturedValues->argObjSyms, insertionPoint, saveInitializedReg); + Assert(!this->func->IsStackArgsEnabled()); // // - We move all argout symbols right before the call so we don't need to restore argouts either @@ -5050,13 +5051,7 @@ IR::Instr* LinearScan::GeneratorBailIn::GenerateBailIn(IR::GeneratorBailInInstr* bailInInstr->capturedValues ); - this->InsertRestoreSymbols( - *bailOutInfo->byteCodeUpwardExposedUsed, - bailInInstr->upwardExposedUses, - bailInInstr->capturedValues, - insertionPoint - ); - Assert(!this->func->IsStackArgsEnabled()); + this->InsertRestoreSymbols(insertionPoint); #ifdef ENABLE_DEBUG_CONFIG_OPTIONS if (PHASE_TRACE(Js::Phase::BailInPhase, this->func)) @@ -5185,8 +5180,6 @@ void LinearScan::GeneratorBailIn::BuildBailInSymbolList( Assert(stackSym); Lifetime* lifetime = stackSym->scratch.linearScan.lifetime; if ( - // Special backend symbols that don't need to be restored - (!stackSym->HasByteCodeRegSlot() && !this->NeedsReloadingBackendSymWhenBailingIn(stackSym)) || // Symbols already in the constant table don't need to be restored either stackSym->IsFromByteCodeConstantTable() || // Symbols having no lifetimes @@ -5201,12 +5194,7 @@ void LinearScan::GeneratorBailIn::BuildBailInSymbolList( AssertOrFailFastMsg(unrestorableSymbols.IsEmpty(), "There are unrestorable backend-only symbols across yield points"); } -void LinearScan::GeneratorBailIn::InsertRestoreSymbols( - const BVSparse& byteCodeUpwardExposedUses, - const BVSparse& upwardExposedUses, - const CapturedValues& capturedValues, - BailInInsertionPoint& insertionPoint -) +void LinearScan::GeneratorBailIn::InsertRestoreSymbols(BailInInsertionPoint& insertionPoint) { FOREACH_SLISTBASE_ENTRY(BailInSymbol, bailInSymbol, this->bailInSymbols) { @@ -5298,18 +5286,6 @@ void LinearScan::GeneratorBailIn::InsertRestoreSymbols( NEXT_SLISTBASE_ENTRY; } -bool LinearScan::GeneratorBailIn::NeedsReloadingBackendSymWhenBailingIn(StackSym* sym) const -{ - // for-in enumerator in generator is loaded as part of the resume jump table. - // By the same reasoning as `initializedRegs`'s, we don't have to restore this whether or not it's been spilled. - if (this->func->GetGeneratorFrameSym() && this->func->GetGeneratorFrameSym()->m_id == sym->m_id) - { - return false; - } - - return true; -} - bool LinearScan::GeneratorBailIn::NeedsReloadingSymWhenBailingIn(StackSym* sym) const { if (sym->IsFromByteCodeConstantTable()) @@ -5325,7 +5301,7 @@ bool LinearScan::GeneratorBailIn::NeedsReloadingSymWhenBailingIn(StackSym* sym) if (!sym->HasByteCodeRegSlot()) { - return this->NeedsReloadingBackendSymWhenBailingIn(sym); + return true; } if (sym->IsConst()) diff --git a/lib/Backend/LinearScan.h b/lib/Backend/LinearScan.h index 51358246c45..90596ecd29d 100644 --- a/lib/Backend/LinearScan.h +++ b/lib/Backend/LinearScan.h @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. +// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- #pragma once @@ -290,18 +291,12 @@ class LinearScan IR::RegOpnd* const interpreterFrameRegOpnd; IR::RegOpnd* const tempRegOpnd; - bool NeedsReloadingBackendSymWhenBailingIn(StackSym* sym) const; bool NeedsReloadingSymWhenBailingIn(StackSym* sym) const; uint32 GetOffsetFromInterpreterStackFrame(Js::RegSlot regSlot) const; IR::SymOpnd* CreateGeneratorObjectOpnd() const; // Insert instructions to restore symbols in the `bailInSymbols` list - void InsertRestoreSymbols( - const BVSparse& bytecodeUpwardExposedUses, - const BVSparse& upwardExposedUses, - const CapturedValues& capturedValues, - BailInInsertionPoint& insertionPoint - ); + void InsertRestoreSymbols(BailInInsertionPoint& insertionPoint); // Fill `bailInSymbols` list with all of the symbols that need to be restored void BuildBailInSymbolList( diff --git a/lib/Backend/SccLiveness.cpp b/lib/Backend/SccLiveness.cpp index ae44fa66c69..ca9698d6282 100644 --- a/lib/Backend/SccLiveness.cpp +++ b/lib/Backend/SccLiveness.cpp @@ -260,9 +260,19 @@ SCCLiveness::Build() { this->EndOpHelper(labelInstr); } - if (labelInstr->isOpHelper && !PHASE_OFF(Js::OpHelperRegOptPhase, this->func)) + if (labelInstr->isOpHelper) { - this->lastOpHelperLabel = labelInstr; + if (!PHASE_OFF(Js::OpHelperRegOptPhase, this->func) && + !this->func->GetTopFunc()->GetJITFunctionBody()->IsCoroutine()) + { + this->lastOpHelperLabel = labelInstr; + } +#ifdef DBG + else + { + labelInstr->AsLabelInstr()->m_noHelperAssert = true; + } +#endif } } else if (instr->IsBranchInstr() && !instr->AsBranchInstr()->IsMultiBranch()) diff --git a/test/es6/generator-jit-bugs.js b/test/es6/generator-jit-bugs.js index 2594cc92d4f..48e499a4775 100644 --- a/test/es6/generator-jit-bugs.js +++ b/test/es6/generator-jit-bugs.js @@ -152,5 +152,21 @@ check(gf8().next().value.v8, 1.1); check(gf8().next().value.v8, 1.1); check(gf8().next().value.v8, 1.1); +// Test 9 - Invalid OpHelperBlockReg spill +title("Inner function access after for...in loop containing yield that is not hit") +{ // Note this bug only occurred when generator was declared inside an outer scope block + let i = 0; + function* gf9() { + function test() {} + for (var unused in []) { + yield undefined; + } + if(++i < 4) + gf9().next() + test + 1; + } +} +check(gf9().next().done, true); + print("pass");