From 27a0c16aea7e66340ecad4042eb3877ce2e6791c Mon Sep 17 00:00:00 2001 From: rhuanjl Date: Sat, 13 Jun 2020 13:56:22 +0100 Subject: [PATCH 1/5] Fix register mismatch in Jit --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index 499051dc77c..9ce25038f25 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -9703,9 +9703,11 @@ void EmitGetAsyncIterator( // Iterable does not have a Symbol.asyncIterator method: attempt to get a sync // iterable and wrap it with an AsyncFromSyncIterator writer->MarkLabel(noAsyncIterator); - EmitGetIterator(resultReg, iterableReg, byteCodeGenerator, funcInfo); - writer->Reg2(Js::OpCode::NewAsyncFromSyncIterator, resultReg, resultReg); - + Js::RegSlot iteratorReg = funcInfo->AcquireTmpRegister(); + EmitGetIterator(iteratorReg, iterableReg, byteCodeGenerator, funcInfo); + writer->Reg2(Js::OpCode::NewAsyncFromSyncIterator, iteratorReg, iteratorReg); + writer->Reg2(Js::OpCode::Ld_A_ReuseLoc, resultReg, iteratorReg); + funcInfo->ReleaseTmpRegister(iteratorReg); byteCodeGenerator->Writer()->MarkLabel(finished); } @@ -10775,9 +10777,18 @@ void EmitYieldStar( writer->MarkLabel(noReturnMethod); if (isAsync) - EmitAwait(resumeValueReg, resumeValueReg, byteCodeGenerator, funcInfo); + { + Js::RegSlot awaitValue = funcInfo->AcquireTmpRegister(); + writer->Reg2(Js::OpCode::Ld_A, awaitValue, resumeValueReg); + + EmitAwait(awaitValue, awaitValue, byteCodeGenerator, funcInfo); + writer->Reg2(Js::OpCode::Ld_A_ReuseLoc, yieldStarReg, awaitValue); + + funcInfo->ReleaseTmpRegister(awaitValue); + } + else + writer->Reg2(Js::OpCode::Ld_A_ReuseLoc, yieldStarReg, resumeValueReg); - writer->Reg2(Js::OpCode::Ld_A, yieldStarReg, resumeValueReg); writer->Br(finishReturn); // Throw case: attempt to call throw From c7ca1c229955ea5cfbb947eba2a0dcfd24da9c45 Mon Sep 17 00:00:00 2001 From: rhuanjl Date: Tue, 23 Mar 2021 21:49:58 +0000 Subject: [PATCH 2/5] 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 --- lib/Backend/Func.cpp | 3 +- lib/Backend/Func.h | 13 +++---- lib/Backend/IRBuilder.cpp | 71 ++++++++++++++------------------------ lib/Backend/IRBuilder.h | 22 ++---------- lib/Backend/LinearScan.cpp | 3 +- 5 files changed, 39 insertions(+), 73 deletions(-) diff --git a/lib/Backend/Func.cpp b/lib/Backend/Func.cpp index 0851014c132..8e25640b935 100644 --- a/lib/Backend/Func.cpp +++ b/lib/Backend/Func.cpp @@ -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. //------------------------------------------------------------------------------------------------------- #include "Backend.h" @@ -149,7 +150,7 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem, , m_forInEnumeratorArrayOffset(-1) , argInsCount(0) , m_globalObjTypeSpecFldInfoArray(nullptr) - , m_forInEnumeratorForGeneratorSym(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 5210754b8b2..f1ca1dff944 100644 --- a/lib/Backend/Func.h +++ b/lib/Backend/Func.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 @@ -1063,21 +1064,21 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece; StackSym* m_loopParamSym; StackSym* m_bailoutReturnValueSym; StackSym* m_hasBailedOutSym; - StackSym* m_forInEnumeratorForGeneratorSym; + StackSym* m_generatorFrameSym; public: StackSym* tempSymDouble; StackSym* tempSymBool; - void SetForInEnumeratorSymForGeneratorSym(StackSym* sym) + void SetGeneratorFrameSym(StackSym* sym) { - Assert(this->m_forInEnumeratorForGeneratorSym == nullptr); - this->m_forInEnumeratorForGeneratorSym = sym; + Assert(this->m_generatorFrameSym == nullptr); + this->m_generatorFrameSym = sym; } - StackSym* GetForInEnumeratorSymForGeneratorSym() const + StackSym* GetGeneratorFrameSym() const { - return this->m_forInEnumeratorForGeneratorSym; + return this->m_generatorFrameSym; } // StackSyms' corresponding getters/setters diff --git a/lib/Backend/IRBuilder.cpp b/lib/Backend/IRBuilder.cpp index 270da0e8a09..1df591ce0b7 100644 --- a/lib/Backend/IRBuilder.cpp +++ b/lib/Backend/IRBuilder.cpp @@ -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. //------------------------------------------------------------------------------------------------------- #include "Backend.h" @@ -1255,7 +1256,7 @@ IRBuilder::EnsureLoopBodyForInEnumeratorArrayOpnd() } IR::Opnd * -IRBuilder::BuildForInEnumeratorOpnd(uint forInLoopLevel) +IRBuilder::BuildForInEnumeratorOpnd(uint forInLoopLevel, uint32 offset) { Assert(forInLoopLevel < this->m_func->GetJITFunctionBody()->GetForInLoopDepth()); if (this->IsLoopBody()) @@ -1270,7 +1271,7 @@ IRBuilder::BuildForInEnumeratorOpnd(uint forInLoopLevel) else if (this->m_func->GetJITFunctionBody()->IsCoroutine()) { return IR::IndirOpnd::New( - this->m_generatorJumpTable.EnsureForInEnumeratorArrayOpnd(), + this->m_generatorJumpTable.BuildForInEnumeratorArrayOpnd(offset), forInLoopLevel * sizeof(Js::ForInObjectEnumerator), TyMachPtr, this->m_func @@ -2949,7 +2950,7 @@ IRBuilder::BuildProfiledReg1Unsigned1(Js::OpCode newOpcode, uint32 offset, Js::R if (newOpcode == Js::OpCode::InitForInEnumerator) { IR::RegOpnd * src1Opnd = this->BuildSrcOpnd(R0); - IR::Opnd * src2Opnd = this->BuildForInEnumeratorOpnd(C1); + IR::Opnd * src2Opnd = this->BuildForInEnumeratorOpnd(C1, offset); IR::Instr *instr = IR::ProfiledInstr::New(Js::OpCode::InitForInEnumerator, nullptr, src1Opnd, src2Opnd, m_func); instr->AsProfiledInstr()->u.profileId = profileId; this->AddInstr(instr, offset); @@ -3084,7 +3085,7 @@ IRBuilder::BuildReg1Unsigned1(Js::OpCode newOpcode, uint offset, Js::RegSlot R0, { IR::Instr *instr = IR::Instr::New(Js::OpCode::InitForInEnumerator, m_func); instr->SetSrc1(this->BuildSrcOpnd(R0)); - instr->SetSrc2(this->BuildForInEnumeratorOpnd(C1)); + instr->SetSrc2(this->BuildForInEnumeratorOpnd(C1, offset)); this->AddInstr(instr, offset); return; } @@ -6935,7 +6936,7 @@ IRBuilder::BuildBrReg1Unsigned1(Js::OpCode newOpcode, uint32 offset) void IRBuilder::BuildBrBReturn(Js::OpCode newOpcode, uint32 offset, Js::RegSlot DestRegSlot, uint32 forInLoopLevel, uint32 targetOffset) { - IR::Opnd *srcOpnd = this->BuildForInEnumeratorOpnd(forInLoopLevel); + IR::Opnd *srcOpnd = this->BuildForInEnumeratorOpnd(forInLoopLevel, offset); IR::RegOpnd * destOpnd = this->BuildDstOpnd(DestRegSlot); IR::BranchInstr * branchInstr = IR::BranchInstr::New(newOpcode, destOpnd, nullptr, srcOpnd, m_func); this->AddBranchInstr(branchInstr, offset, targetOffset); @@ -7922,14 +7923,12 @@ IRBuilder::GeneratorJumpTable::BuildJumpTable() // // s1 = Ld_A prm1 // s2 = Ld_A s1[offset of JavascriptGenerator::frame] - // BrNotAddr_A s2 !nullptr $initializationCode + // BrNotAddr_A s2 !nullptr $jumpTable // // $createInterpreterStackFrame: // call helper // - // $initializationCode: - // load for-in enumerator address from interpreter stack frame - // + // Br $startOfFunc // // $jumpTable: // @@ -7963,23 +7962,25 @@ IRBuilder::GeneratorJumpTable::BuildJumpTable() IR::LabelInstr* functionBegin = IR::LabelInstr::New(Js::OpCode::Label, this->m_func); LABELNAMESET(functionBegin, "GeneratorFunctionBegin"); - IR::LabelInstr* initCode = IR::LabelInstr::New(Js::OpCode::Label, this->m_func); - LABELNAMESET(initCode, "GeneratorInitializationAndJumpTable"); + IR::LabelInstr* jumpTable = IR::LabelInstr::New(Js::OpCode::Label, this->m_func); + LABELNAMESET(jumpTable, "GeneratorJumpTable"); - // BrNotAddr_A s2 nullptr $initializationCode - IR::BranchInstr* skipCreateInterpreterFrame = IR::BranchInstr::New(Js::OpCode::BrNotAddr_A, initCode, genFrameOpnd, IR::AddrOpnd::NewNull(this->m_func), this->m_func); + // If there is already a stack frame, generator function has previously begun execution - don't recreate, skip down to jump table + // BrNotAddr_A s2 nullptr $jumpTable + IR::BranchInstr* skipCreateInterpreterFrame = IR::BranchInstr::New(Js::OpCode::BrNotAddr_A, jumpTable, genFrameOpnd, IR::AddrOpnd::NewNull(this->m_func), this->m_func); this->m_irBuilder->AddInstr(skipCreateInterpreterFrame, this->m_irBuilder->m_functionStartOffset); // Create interpreter stack frame IR::Instr* createInterpreterFrame = IR::Instr::New(Js::OpCode::GeneratorCreateInterpreterStackFrame, genFrameOpnd /* dst */, genRegOpnd /* src */, this->m_func); this->m_irBuilder->AddInstr(createInterpreterFrame, this->m_irBuilder->m_functionStartOffset); + // Having created the frame, skip over the jump table and start executing from the beginning of the function IR::BranchInstr* skipJumpTable = IR::BranchInstr::New(Js::OpCode::Br, functionBegin, this->m_func); this->m_irBuilder->AddInstr(skipJumpTable, this->m_irBuilder->m_functionStartOffset); - // Label to insert any initialization code - // $initializationCode: - this->m_irBuilder->AddInstr(initCode, this->m_irBuilder->m_functionStartOffset); + // Label for start of jumpTable - where we look for the correct Yield resume point + // $jumpTable: + this->m_irBuilder->AddInstr(jumpTable, this->m_irBuilder->m_functionStartOffset); // s3 = Ld_A s2[offset of InterpreterStackFrame::m_reader.m_currentLocation] IR::RegOpnd* curLocOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func); @@ -8015,46 +8016,24 @@ IRBuilder::GeneratorJumpTable::BuildJumpTable() this->m_irBuilder->AddInstr(functionBegin, this->m_irBuilder->m_functionStartOffset); - // Save these values for later use - this->m_initLabel = initCode; + // Save this value for later use this->m_generatorFrameOpnd = genFrameOpnd; + this->m_func->SetGeneratorFrameSym(genFrameOpnd->GetStackSym()); return this->m_irBuilder->m_lastInstr; } -IR::LabelInstr* -IRBuilder::GeneratorJumpTable::GetInitLabel() const -{ - Assert(this->m_initLabel != nullptr); - return this->m_initLabel; -} - IR::RegOpnd* -IRBuilder::GeneratorJumpTable::CreateForInEnumeratorArrayOpnd() -{ - Assert(this->m_initLabel != nullptr); +IRBuilder::GeneratorJumpTable::BuildForInEnumeratorArrayOpnd(uint32 offset) +{ Assert(this->m_generatorFrameOpnd != nullptr); - IR::RegOpnd* forInEnumeratorOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func); - IR::Instr* instr = IR::Instr::New( - Js::OpCode::Ld_A, - forInEnumeratorOpnd, + 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), this->m_func ); - this->m_initLabel->InsertAfter(instr); - - return forInEnumeratorOpnd; -} - -IR::RegOpnd* -IRBuilder::GeneratorJumpTable::EnsureForInEnumeratorArrayOpnd() -{ - if (this->m_forInEnumeratorArrayOpnd == nullptr) - { - this->m_forInEnumeratorArrayOpnd = this->CreateForInEnumeratorArrayOpnd(); - this->m_func->SetForInEnumeratorSymForGeneratorSym(m_forInEnumeratorArrayOpnd->GetStackSym()); - } + this->m_irBuilder->AddInstr(instr, offset); - return this->m_forInEnumeratorArrayOpnd; + return forInEnumeratorArrayOpnd; } diff --git a/lib/Backend/IRBuilder.h b/lib/Backend/IRBuilder.h index ddfe0171eae..6f64831eb8b 100644 --- a/lib/Backend/IRBuilder.h +++ b/lib/Backend/IRBuilder.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 @@ -227,7 +228,7 @@ class IRBuilder IR::RegOpnd * BuildSrcOpnd(Js::RegSlot srcRegSlot, IRType type = TyVar); IR::AddrOpnd * BuildAuxArrayOpnd(AuxArrayValue auxArrayType, uint32 auxArrayOffset); IR::Opnd * BuildAuxObjectLiteralTypeRefOpnd(int objectId); - IR::Opnd * BuildForInEnumeratorOpnd(uint forInLoopLevel); + IR::Opnd * BuildForInEnumeratorOpnd(uint forInLoopLevel, uint32 offset); IR::RegOpnd * EnsureLoopBodyForInEnumeratorArrayOpnd(); private: uint AddStatementBoundary(uint statementIndex, uint offset); @@ -366,29 +367,12 @@ class IRBuilder Func* const m_func; IRBuilder* const m_irBuilder; - // for-in enumerators are allocated on the heap for jit'd loop body - // and on the stack for all other cases (the interpreter frame will - // reuses them when bailing out). But because we have the concept of - // "bailing in" for generator, reusing enumerators allocated on the stack - // would not work. So we have to allocate them on the generator's interpreter - // frame instead. This operand is loaded as part of the jump table, before we - // jump to any of the resume point. - IR::RegOpnd* m_forInEnumeratorArrayOpnd = nullptr; - IR::RegOpnd* m_generatorFrameOpnd = nullptr; - // This label is used to insert any initialization code that might be needed - // when bailing in and before jumping to any of the resume points. - // As of now, we only need to load the operand for for-in enumerator. - IR::LabelInstr* m_initLabel = nullptr; - - IR::RegOpnd* CreateForInEnumeratorArrayOpnd(); - public: GeneratorJumpTable(Func* func, IRBuilder* irBuilder); IR::Instr* BuildJumpTable(); - IR::LabelInstr* GetInitLabel() const; - IR::RegOpnd* EnsureForInEnumeratorArrayOpnd(); + IR::RegOpnd* BuildForInEnumeratorArrayOpnd(uint32 offset); }; GeneratorJumpTable m_generatorJumpTable; diff --git a/lib/Backend/LinearScan.cpp b/lib/Backend/LinearScan.cpp index ff88cae6261..3f852a87d44 100644 --- a/lib/Backend/LinearScan.cpp +++ b/lib/Backend/LinearScan.cpp @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft Corporation and contributors. 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. //------------------------------------------------------------------------------------------------------- @@ -5301,7 +5302,7 @@ bool LinearScan::GeneratorBailIn::NeedsReloadingBackendSymWhenBailingIn(StackSym { // 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->GetForInEnumeratorSymForGeneratorSym() && this->func->GetForInEnumeratorSymForGeneratorSym()->m_id == sym->m_id) + if (this->func->GetGeneratorFrameSym() && this->func->GetGeneratorFrameSym()->m_id == sym->m_id) { return false; } From c86df3fc3c61afad28aa49d19a97117693deae55 Mon Sep 17 00:00:00 2001 From: rhuanjl Date: Tue, 23 Mar 2021 21:53:59 +0000 Subject: [PATCH 3/5] Disable BailOnNoProfile in Generators/Asyncs - BailOnNoProfile marks all code after it as dead - if variable is used in a yield BUT incremented after BailOnNoProfile - Jit would delete the increment and mark variable as constant - interpreter would increment BUT JIT would reload as const - would yield same value multiple times Ideally should introduce Generator/Async specific version of BailOnNoProfile That doesn't mark code after it as dead - but that would be larger work. For now disable BailOnNoProfile for generators/Asyncs. --- lib/Backend/IRBuilder.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/Backend/IRBuilder.cpp b/lib/Backend/IRBuilder.cpp index 1df591ce0b7..18ab4848fb4 100644 --- a/lib/Backend/IRBuilder.cpp +++ b/lib/Backend/IRBuilder.cpp @@ -118,6 +118,11 @@ IRBuilder::DoBailOnNoProfile() return false; } + if (m_func->GetTopFunc()->GetJITFunctionBody()->IsCoroutine()) + { + return false; + } + return true; } From 290a3a81db73042f6770febd275d2d1f66eebe94 Mon Sep 17 00:00:00 2001 From: rhuanjl Date: Tue, 23 Mar 2021 22:00:51 +0000 Subject: [PATCH 4/5] Remove unnecessary startup yield - Extra yield inserted at the top of Generator functions - so Params with side effects can be executed upon function call - then stop at the yield untill .next() is used - in cases where there are no params with side effets his is not needed - use a condition to only do it when needed - saves several extra ops - Had to update TTD code for this (it expected all Gens to have state) - AsyncGenerator parameters needed copying to heap on call previously this was done when executing until the startup yield --- lib/Runtime/Base/FunctionInfo.h | 5 ++++- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 8 ++++---- lib/Runtime/ByteCode/ByteCodeGenerator.cpp | 8 +++++++- lib/Runtime/Debug/TTSnapObjects.cpp | 3 ++- .../Library/JavascriptAsyncGenerator.cpp | 17 +++++++++++++++-- .../JavascriptAsyncGeneratorFunction.cpp | 2 ++ .../Library/JavascriptGeneratorFunction.cpp | 3 +++ 7 files changed, 37 insertions(+), 9 deletions(-) diff --git a/lib/Runtime/Base/FunctionInfo.h b/lib/Runtime/Base/FunctionInfo.h index 4d01e771bd2..98eee1bea03 100644 --- a/lib/Runtime/Base/FunctionInfo.h +++ b/lib/Runtime/Base/FunctionInfo.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. //------------------------------------------------------------------------------------------------------- namespace Js @@ -42,7 +43,8 @@ namespace Js Method = 0x400000, // The function is a method ComputedName = 0x800000, ActiveScript = 0x1000000, - HomeObj = 0x2000000 + HomeObj = 0x2000000, + GeneratorWithComplexParams = 0x8000000 // Generator function with non-simple params needs startup yield }; FunctionInfo(JavascriptMethod entryPoint, Attributes attributes = None, LocalFunctionId functionId = Js::Constants::NoFunctionId, FunctionProxy* functionBodyImpl = nullptr); FunctionInfo(JavascriptMethod entryPoint, _no_write_barrier_tag, Attributes attributes = None, LocalFunctionId functionId = Js::Constants::NoFunctionId, FunctionProxy* functionBodyImpl = nullptr); @@ -136,6 +138,7 @@ namespace Js bool GetBaseConstructorKind() const { return (attributes & Attributes::BaseConstructorKind) != 0; } bool IsActiveScript() const { return ((this->attributes & Attributes::ActiveScript) != 0); } void SetIsActiveScript() { attributes = (Attributes)(attributes | Attributes::ActiveScript); } + bool GetGeneratorWithComplexParams() {return (attributes & Attributes::GeneratorWithComplexParams) != 0; } protected: FieldNoBarrier(JavascriptMethod) originalEntryPoint; FieldWithBarrier(FunctionProxy *) functionBodyImpl; // Implementation of the function- null if the function doesn't have a body diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index 9ce25038f25..9f24d96052e 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -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. //------------------------------------------------------------------------------------------------------- #include "RuntimeByteCodePch.h" @@ -3023,10 +3024,9 @@ void ByteCodeGenerator::EmitOneFunction(ParseNodeFnc *pnodeFnc) } // If the function has non simple parameter list, the params needs to be evaluated when the generator object is created - // (that is when the function is called). This yield opcode is to mark the begining of the function body. - // TODO: Inserting a yield should have almost no impact on perf as it is a direct return from the function. But this needs - // to be verified. Ideally if the function has simple parameter list then we can avoid inserting the opcode and the additional call. - if (pnodeFnc->IsGenerator()) + // (that is when the function is called). So insert an extra yield we can execute up to, to do this. + // In a Module we execute until this yield to hoist functions accross modules. + if (pnodeFnc->IsGenerator() && (pnodeFnc->HasNonSimpleParameterList() || pnodeFnc->IsModule())) { EmitStartupYield(this, funcInfo); } diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp index c859ef631fd..1ed46444cf4 100644 --- a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp +++ b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp @@ -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. //------------------------------------------------------------------------------------------------------- #include "RuntimeByteCodePch.h" @@ -1240,7 +1241,8 @@ static const Js::FunctionInfo::Attributes StableFunctionInfoAttributesMask = (Js Js::FunctionInfo::Attributes::Generator | Js::FunctionInfo::Attributes::Module | Js::FunctionInfo::Attributes::ComputedName | - Js::FunctionInfo::Attributes::HomeObj + Js::FunctionInfo::Attributes::HomeObj | + Js::FunctionInfo::Attributes::GeneratorWithComplexParams ); static Js::FunctionInfo::Attributes GetFunctionInfoAttributes(ParseNodeFnc * pnodeFnc) @@ -1289,6 +1291,10 @@ static Js::FunctionInfo::Attributes GetFunctionInfoAttributes(ParseNodeFnc * pno if (pnodeFnc->IsGenerator()) { attributes = (Js::FunctionInfo::Attributes)(attributes | Js::FunctionInfo::Attributes::Generator); + if (pnodeFnc->HasNonSimpleParameterList()) + { + attributes = (Js::FunctionInfo::Attributes)(attributes | Js::FunctionInfo::Attributes::GeneratorWithComplexParams); + } } if (pnodeFnc->IsAccessor()) { diff --git a/lib/Runtime/Debug/TTSnapObjects.cpp b/lib/Runtime/Debug/TTSnapObjects.cpp index 44c53bb7696..7105d9ef0d5 100644 --- a/lib/Runtime/Debug/TTSnapObjects.cpp +++ b/lib/Runtime/Debug/TTSnapObjects.cpp @@ -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. //------------------------------------------------------------------------------------------------------- #include "RuntimeDebugPch.h" @@ -2324,9 +2325,9 @@ namespace TTD } } - generator->SetFrameSlots(generatorInfo->frame_slotCount, frameSlotArray); if (generatorInfo->byteCodeReader_offset > 0) { + generator->SetFrameSlots(generatorInfo->frame_slotCount, frameSlotArray); frame->InitializeClosures(); frame->GetReader()->SetCurrentOffset(generatorInfo->byteCodeReader_offset); } diff --git a/lib/Runtime/Library/JavascriptAsyncGenerator.cpp b/lib/Runtime/Library/JavascriptAsyncGenerator.cpp index 1d15b552a32..a1c6b31bd0d 100644 --- a/lib/Runtime/Library/JavascriptAsyncGenerator.cpp +++ b/lib/Runtime/Library/JavascriptAsyncGenerator.cpp @@ -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. //------------------------------------------------------------------------------------------------------- #include "RuntimeLibraryPch.h" @@ -13,13 +14,25 @@ JavascriptAsyncGenerator* JavascriptAsyncGenerator::New( Arguments& args, ScriptFunction* scriptFunction) { + // InterpreterStackFrame takes a pointer to the args, so copy them to the recycler + // heap and use that buffer for the asyncgenerator's InterpreterStackFrame + Field(Var)* argValuesCopy = nullptr; + + if (args.Info.Count > 0) + { + argValuesCopy = RecyclerNewArray(recycler, Field(Var), args.Info.Count); + CopyArray(argValuesCopy, args.Info.Count, args.Values, args.Info.Count); + } + + Arguments heapArgs(args.Info, unsafe_write_barrier_cast(argValuesCopy)); + auto* requestQueue = RecyclerNew(recycler, JavascriptAsyncGenerator::RequestQueue, recycler); - JavascriptAsyncGenerator* generator = RecyclerNew( + JavascriptAsyncGenerator* generator = RecyclerNewFinalized( recycler, JavascriptAsyncGenerator, generatorType, - args, + heapArgs, scriptFunction, requestQueue); diff --git a/lib/Runtime/Library/JavascriptAsyncGeneratorFunction.cpp b/lib/Runtime/Library/JavascriptAsyncGeneratorFunction.cpp index 949a6f17bf4..451c67465e1 100644 --- a/lib/Runtime/Library/JavascriptAsyncGeneratorFunction.cpp +++ b/lib/Runtime/Library/JavascriptAsyncGeneratorFunction.cpp @@ -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. //------------------------------------------------------------------------------------------------------- #include "RuntimeLibraryPch.h" @@ -55,6 +56,7 @@ Var JavascriptAsyncGeneratorFunction::EntryAsyncGeneratorFunctionImplementation( auto* generator = library->CreateAsyncGenerator(args, scriptFn, prototype); // Run the generator to execute until the beginning of the body + if (scriptFn->GetFunctionInfo()->GetGeneratorWithComplexParams()) BEGIN_SAFE_REENTRANT_CALL(scriptContext->GetThreadContext()) { generator->CallGenerator(library->GetUndefined(), ResumeYieldKind::Normal); diff --git a/lib/Runtime/Library/JavascriptGeneratorFunction.cpp b/lib/Runtime/Library/JavascriptGeneratorFunction.cpp index f5ff7f046ae..fb7bcbb11b5 100644 --- a/lib/Runtime/Library/JavascriptGeneratorFunction.cpp +++ b/lib/Runtime/Library/JavascriptGeneratorFunction.cpp @@ -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. //------------------------------------------------------------------------------------------------------- #include "RuntimeLibraryPch.h" @@ -120,6 +121,8 @@ using namespace Js; prototype); // Call a next on the generator to execute till the beginning of the body + FunctionInfo* funcInfo = generatorFunction->scriptFunction->GetFunctionInfo(); + if (funcInfo->GetGeneratorWithComplexParams() || funcInfo->IsModule()) BEGIN_SAFE_REENTRANT_CALL(scriptContext->GetThreadContext()) { generator->CallGenerator(library->GetUndefined(), ResumeYieldKind::Normal); From a4d705a879d4e82592a063defb314ba05c5d522e Mon Sep 17 00:00:00 2001 From: rhuanjl Date: Tue, 23 Mar 2021 22:05:33 +0000 Subject: [PATCH 5/5] Enable Jitting Generators and Async Functions - exclude ARM which has not been tested - excludes code running with debugger attached (not tested) - excludes functions containing try/catch (not tested) - excludes Module globals (unlikely to benefit) - add tests for bugs fixed above --- lib/Common/ConfigFlagsList.h | 11 +++- lib/Runtime/Base/FunctionBody.cpp | 11 ++-- test/es6/generator-jit-bugs.js | 85 ++++++++++++++++++++++++++++--- test/es6/rlexe.xml | 16 +++++- 4 files changed, 109 insertions(+), 14 deletions(-) diff --git a/lib/Common/ConfigFlagsList.h b/lib/Common/ConfigFlagsList.h index b32993f8482..6581ad2a8fb 100644 --- a/lib/Common/ConfigFlagsList.h +++ b/lib/Common/ConfigFlagsList.h @@ -673,6 +673,15 @@ PHASE(All) #define DEFAULT_CONFIG_ESArrayFindFromLast (false) #define DEFAULT_CONFIG_ESNullishCoalescingOperator (true) #define DEFAULT_CONFIG_ESGlobalThis (true) + +// Jitting generators has not been tested on ARM +// enabled only for x86 and x64 for now +#ifdef _M_ARM32_OR_ARM64 + #define DEFAULT_CONFIG_JitES6Generators (false) +#else + #define DEFAULT_CONFIG_JitES6Generators (true) +#endif + #ifdef COMPILE_DISABLE_ES6RegExPrototypeProperties // If ES6RegExPrototypeProperties needs to be disabled by compile flag, DEFAULT_CONFIG_ES6RegExPrototypeProperties should be false #define DEFAULT_CONFIG_ES6RegExPrototypeProperties (false) @@ -1206,7 +1215,7 @@ FLAGR(Boolean, ESImportMeta, "Enable import.meta keyword", DEFAULT_CONFIG_ESImpo FLAGR(Boolean, ESGlobalThis, "Enable globalThis", DEFAULT_CONFIG_ESGlobalThis) // This flag to be removed once JITing generator functions is stable -FLAGNR(Boolean, JitES6Generators , "Enable JITing of ES6 generators", false) +FLAGNR(Boolean, JitES6Generators , "Enable JITing of ES6 generators", DEFAULT_CONFIG_JitES6Generators) FLAGNR(Boolean, FastLineColumnCalculation, "Enable fast calculation of line/column numbers from the source.", DEFAULT_CONFIG_FastLineColumnCalculation) FLAGR (String, Filename , "Jscript source file", nullptr) diff --git a/lib/Runtime/Base/FunctionBody.cpp b/lib/Runtime/Base/FunctionBody.cpp index 833e20c1843..5f625fc7be7 100644 --- a/lib/Runtime/Base/FunctionBody.cpp +++ b/lib/Runtime/Base/FunctionBody.cpp @@ -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. //------------------------------------------------------------------------------------------------------- #include "RuntimeBasePch.h" @@ -341,13 +342,13 @@ namespace Js bool FunctionBody::SkipAutoProfileForCoroutine() const { - return this->IsCoroutine() && CONFIG_ISENABLED(Js::JitES6GeneratorsFlag); + return this->IsCoroutine() && CONFIG_FLAG(JitES6Generators); } bool FunctionBody::IsGeneratorAndJitIsDisabled() const { - return this->IsCoroutine() && !(CONFIG_ISENABLED(Js::JitES6GeneratorsFlag) && !this->GetHasTry() && !this->IsInDebugMode() && !this->IsAsync()); + return this->IsCoroutine() && !(CONFIG_FLAG(JitES6Generators) && !this->GetHasTry() && !this->IsInDebugMode() && !this->IsModule()); } ScriptContext* EntryPointInfo::GetScriptContext() @@ -7151,8 +7152,7 @@ namespace Js !GetScriptContext()->IsScriptContextInDebugMode() && DoInterpreterProfile() && #pragma warning(suppress: 6235) // ( || ) is always a non-zero constant. - (!CONFIG_FLAG(NewSimpleJit) || DoInterpreterAutoProfile()) && - !IsCoroutine(); // Generator JIT requires bailout which SimpleJit cannot do since it skips GlobOpt + (!CONFIG_FLAG(NewSimpleJit) || DoInterpreterAutoProfile()); } bool FunctionBody::DoSimpleJitWithLock() const @@ -7166,8 +7166,7 @@ namespace Js !this->IsInDebugMode() && DoInterpreterProfileWithLock() && #pragma warning(suppress: 6235) // ( || ) is always a non-zero constant. - (!CONFIG_FLAG(NewSimpleJit) || DoInterpreterAutoProfile()) && - !IsCoroutine(); // Generator JIT requires bailout which SimpleJit cannot do since it skips GlobOpt + (!CONFIG_FLAG(NewSimpleJit) || DoInterpreterAutoProfile()); } bool FunctionBody::DoSimpleJitDynamicProfile() const diff --git a/test/es6/generator-jit-bugs.js b/test/es6/generator-jit-bugs.js index 676afe058d0..c3035de0834 100644 --- a/test/es6/generator-jit-bugs.js +++ b/test/es6/generator-jit-bugs.js @@ -1,20 +1,93 @@ //------------------------------------------------------------------------------------------------------- // 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. //------------------------------------------------------------------------------------------------------- +let results = 0; +let test = 0; +const verbose = WScript.Arguments[0] != "summary"; + +function check(actual, expected) { + if (actual != expected) + throw new Error("Generator produced " + actual + " instead of " + expected); + if (verbose) + print('Result ' + ++results + ' Generator correctly produced ' + actual); +} + +function title (name) { + if (verbose) { + print("Beginning Test " + ++test + ": " + name); + results = 0; + } +} + + // Test 1 - const that is unused/replaced with undefined -function* foo() { +title("const that is unused/replaced with undefined"); +function* gf1() { const temp2 = null; while (true) { yield temp2; } } -const gen = foo(); +const gen = gf1(); + +check(gen.next().value, null); +check(gen.next().value, null); +check(gen.next().value, null); + +// Test 2 - load for-in enumerator in nested for-in loop +title("load for-in enumerator in nested for-in loop"); +const arr = [0, 1, 2]; +function* gf2() { + for (let a in arr) { + for (let b in arr) { + yield a + b; + } + } +} + +const gen2 = gf2(); + +check(gen2.next().value, "00"); +check(gen2.next().value, "01"); +check(gen2.next().value, "02"); +check(gen2.next().value, "10"); +check(gen2.next().value, "11"); +check(gen2.next().value, "12"); +check(gen2.next().value, "20"); +check(gen2.next().value, "21"); +check(gen2.next().value, "22"); +check(gen2.next().value, undefined); + +// Test 3 - Bail on no profile losing loop control variable +title("Bail on no profile losing loop control variable"); +function* gf3() { + for (let i = 0; i < 3; ++i) { + yield i; + } +} + +const gen3 = gf3(); + +check(gen3.next().value, 0); +check(gen3.next().value, 1); +check(gen3.next().value, 2); + +// Test 4 - yield* iterator fails to be restored after Bail on No Profile +title("Bail on no profile losing yield* iterator") +function* gf4() { + yield 0; + yield* [1,2,3]; +} + +const gen4 = gf4(); -gen.next(); -gen.next(); -gen.next(); +check(gen4.next().value, 0); +check(gen4.next().value, 1); +check(gen4.next().value, 2); +check(gen4.next().value, 3); -print("Pass"); +print("pass"); diff --git a/test/es6/rlexe.xml b/test/es6/rlexe.xml index 0c465d79d6d..e079f4c46eb 100644 --- a/test/es6/rlexe.xml +++ b/test/es6/rlexe.xml @@ -135,10 +135,24 @@ generator-jit-bugs.js - -JitES6Generators + -JitES6Generators -args summary -endargs exclude_nonative + + + generator-jit-bugs.js + -JitES6Generators -off:simplejit -args summary -endargs + exclude_nonative + + + + + generator-jit-bugs.js + -JitES6Generators -off:fulljit -args summary -endargs + exclude_nonative, exclude_dynapogo + + proto_basic.js