From 5e72ccfa7d389a52eb39aaad01393c4fc31581fb Mon Sep 17 00:00:00 2001 From: rhuanjl Date: Sun, 12 Oct 2025 16:01:21 +0100 Subject: [PATCH] Avoid using TryFinallyWithYield OpCode when there is no Yield. Implement partial tracking of Yield within ByteCodeGenerator to enable this. The OpCode TryFinallyWithYield is not supported by the Jit. It was being used unnecessarily in various code patterns that should be safe to Jit, such patterns were hitting an Abort in the Jit because of the unimplemented OpCode. As the OpCode is unnecessary when there is no Yield (the cases we currently try to jit) fix logic so we do not use it in those cases. --- lib/Backend/Lower.cpp | 3 + lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 34 +++++---- lib/Runtime/ByteCode/ByteCodeGenerator.cpp | 62 ++++++++++++++++ lib/Runtime/ByteCode/ByteCodeGenerator.h | 7 ++ test/es6GeneratorJit/async-jit-bugs.js | 82 ++++++++++++++++++++-- 5 files changed, 170 insertions(+), 18 deletions(-) diff --git a/lib/Backend/Lower.cpp b/lib/Backend/Lower.cpp index b65b1671ec4..9398bc6270b 100644 --- a/lib/Backend/Lower.cpp +++ b/lib/Backend/Lower.cpp @@ -2605,6 +2605,9 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa instrPrev = this->LowerTry(instr, true /*try-catch*/); break; + case Js::OpCode::TryFinallyWithYield: + // TODO Implement TryFinallyWithYield before enabling Jit on full async functions + AssertOrFailFastMsg(false, "TryFinallyWithYield not implemented in Jit, should not be trying to Jit this function."); case Js::OpCode::TryFinally: instrPrev = this->LowerTry(instr, false /*try-finally*/); break; diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index 98533e8f0e6..4dd74a79589 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -6777,9 +6777,9 @@ void EmitIteratorTopLevelFinally( Js::RegSlot yieldOffsetLocation, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, + bool hasYield, bool isAsync) { - bool isCoroutine = funcInfo->byteCodeFunction->IsCoroutine(); Js::ByteCodeLabel afterFinallyBlockLabel = byteCodeGenerator->Writer()->DefineLabel(); byteCodeGenerator->Writer()->Empty(Js::OpCode::Leave); @@ -6805,8 +6805,9 @@ void EmitIteratorTopLevelFinally( byteCodeGenerator->Writer()->MarkLabel(skipCallCloseLabel); byteCodeGenerator->PopJumpCleanup(); - if (isCoroutine) + if (hasYield) { + Assert(funcInfo->byteCodeFunction->IsCoroutine()); funcInfo->ReleaseTmpRegister(yieldOffsetLocation); funcInfo->ReleaseTmpRegister(yieldExceptionLocation); } @@ -6826,6 +6827,7 @@ void EmitIteratorCatchAndFinally( Js::RegSlot yieldOffsetLocation, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, + bool hasYield, bool isAsync = false) { byteCodeGenerator->PopJumpCleanup(); @@ -6849,6 +6851,7 @@ void EmitIteratorCatchAndFinally( yieldOffsetLocation, byteCodeGenerator, funcInfo, + hasYield, isAsync); funcInfo->ReleaseTmpRegister(shouldCallReturnFunctionLocationFinally); @@ -6901,10 +6904,11 @@ void EmitDestructuredArray( Js::RegSlot regException = Js::Constants::NoRegister; Js::RegSlot regOffset = Js::Constants::NoRegister; - bool isCoroutine = funcInfo->byteCodeFunction->IsCoroutine(); + bool hasYield = byteCodeGenerator->GetHasYield(lhs); - if (isCoroutine) + if (hasYield) { + Assert(funcInfo->byteCodeFunction->IsCoroutine()); regException = funcInfo->AcquireTmpRegister(); regOffset = funcInfo->AcquireTmpRegister(); } @@ -6914,7 +6918,7 @@ void EmitDestructuredArray( Js::ByteCodeLabel catchLabel = byteCodeGenerator->Writer()->DefineLabel(); byteCodeGenerator->Writer()->RecordCrossFrameEntryExitRecord(true); - if (isCoroutine) + if (hasYield) { byteCodeGenerator->Writer()->BrReg2(Js::OpCode::TryFinallyWithYield, finallyLabel, regException, regOffset); byteCodeGenerator->PushJumpCleanupForTry( @@ -6950,7 +6954,8 @@ void EmitDestructuredArray( regException, regOffset, byteCodeGenerator, - funcInfo); + funcInfo, + hasYield); funcInfo->ReleaseTmpRegister(nextMethodReg); funcInfo->ReleaseTmpRegister(iteratorLocation); @@ -9836,6 +9841,7 @@ void EmitForInOrForOf(ParseNodeForInOrForOf *loopNode, ByteCodeGenerator *byteCo { bool isForIn = (loopNode->nop == knopForIn); bool isForAwaitOf = (loopNode->nop == knopForAwaitOf); + bool hasYield = isForAwaitOf || byteCodeGenerator->GetHasYield(loopNode); Assert(isForAwaitOf || isForIn || loopNode->nop == knopForOf); BeginEmitBlock(loopNode->pnodeBlock, byteCodeGenerator, funcInfo); @@ -9899,10 +9905,9 @@ void EmitForInOrForOf(ParseNodeForInOrForOf *loopNode, ByteCodeGenerator *byteCo Js::RegSlot shouldCallReturnFunctionLocation = loopNode->shouldCallReturnFunctionLocation; Js::RegSlot shouldCallReturnFunctionLocationFinally = loopNode->shouldCallReturnFunctionLocationFinally; - bool isCoroutine = funcInfo->byteCodeFunction->IsCoroutine(); - - if (isCoroutine) + if (hasYield) { + Assert(funcInfo->byteCodeFunction->IsCoroutine()); regException = funcInfo->AcquireTmpRegister(); regOffset = funcInfo->AcquireTmpRegister(); } @@ -9946,7 +9951,7 @@ void EmitForInOrForOf(ParseNodeForInOrForOf *loopNode, ByteCodeGenerator *byteCo byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdFalse, shouldCallReturnFunctionLocation); byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdFalse, shouldCallReturnFunctionLocationFinally); - if (isCoroutine) + if (hasYield) { byteCodeGenerator->Writer()->BrReg2(Js::OpCode::TryFinallyWithYield, finallyLabel, regException, regOffset); byteCodeGenerator->PushJumpCleanupForTry( @@ -10027,6 +10032,7 @@ void EmitForInOrForOf(ParseNodeForInOrForOf *loopNode, ByteCodeGenerator *byteCo regOffset, byteCodeGenerator, funcInfo, + hasYield, isForAwaitOf); } @@ -12545,11 +12551,11 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func finallyLabel = byteCodeGenerator->Writer()->DefineLabel(); byteCodeGenerator->Writer()->RecordCrossFrameEntryExitRecord(true); + bool hasYield = byteCodeGenerator->GetHasYield(pnodeTryFinally); - // [CONSIDER][aneeshd] Ideally the TryFinallyWithYield opcode needs to be used only if there is a yield expression. - // For now, if the function is generator we are using the TryFinallyWithYield. - if (funcInfo->byteCodeFunction->IsCoroutine()) + if (hasYield) { + Assert(funcInfo->byteCodeFunction->IsCoroutine()); regException = funcInfo->AcquireTmpRegister(); regOffset = funcInfo->AcquireTmpRegister(); byteCodeGenerator->Writer()->BrReg2(Js::OpCode::TryFinallyWithYield, finallyLabel, regException, regOffset); @@ -12593,7 +12599,7 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func Emit(pnodeFinally->pnodeBody, byteCodeGenerator, funcInfo, fReturnValue); funcInfo->ReleaseLoc(pnodeFinally->pnodeBody); - if (funcInfo->byteCodeFunction->IsCoroutine()) + if (hasYield) { funcInfo->ReleaseTmpRegister(regOffset); funcInfo->ReleaseTmpRegister(regException); diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp index 84aaf5c6ba0..234b68e5903 100644 --- a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp +++ b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp @@ -219,6 +219,12 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref prefix(pnode, byteCodeGenerator); switch (pnode->nop) { + case knopYield: + case knopYieldLeaf: + case knopYieldStar: + case knopAwait: + byteCodeGenerator->SetHasYield(); + // fall through to default default: { uint flags = ParseNode::Grfnop(pnode->nop); @@ -245,6 +251,7 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref break; case knopArrayPattern: + byteCodeGenerator->PushTrackForYield(pnode); if (!byteCodeGenerator->InDestructuredPattern()) { byteCodeGenerator->SetInDestructuredPattern(true); @@ -255,6 +262,7 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref { Visit(pnode->AsParseNodeUni()->pnode1, byteCodeGenerator, prefix, postfix); } + byteCodeGenerator->PopTrackForYield(pnode); break; case knopCall: @@ -379,11 +387,13 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref case knopForOf: case knopForAwaitOf: BeginVisitBlock(pnode->AsParseNodeForInOrForOf()->pnodeBlock, byteCodeGenerator); + byteCodeGenerator->PushTrackForYield(pnode); Visit(pnode->AsParseNodeForInOrForOf()->pnodeLval, byteCodeGenerator, prefix, postfix); Visit(pnode->AsParseNodeForInOrForOf()->pnodeObj, byteCodeGenerator, prefix, postfix); byteCodeGenerator->EnterLoop(); Visit(pnode->AsParseNodeForInOrForOf()->pnodeBody, byteCodeGenerator, prefix, postfix, pnode); byteCodeGenerator->ExitLoop(); + byteCodeGenerator->PopTrackForYield(pnode); EndVisitBlock(pnode->AsParseNodeForInOrForOf()->pnodeBlock, byteCodeGenerator); break; // PTNODE(knopReturn , "return" ,None ,Uni ,fnopNone) @@ -441,8 +451,10 @@ void Visit(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, PrefixFn pref break; // PTNODE(knopTryCatchFinally,"try-catch-finally",None,TryCatchFinally,fnopCleanup) case knopTryFinally: + byteCodeGenerator->PushTrackForYield(pnode); Visit(pnode->AsParseNodeTryFinally()->pnodeTry, byteCodeGenerator, prefix, postfix, pnode); Visit(pnode->AsParseNodeTryFinally()->pnodeFinally, byteCodeGenerator, prefix, postfix, pnode); + byteCodeGenerator->PopTrackForYield(pnode); break; // PTNODE(knopTryCatch , "try-catch" ,None ,TryCatch ,fnopCleanup) case knopTryCatch: @@ -728,6 +740,8 @@ ByteCodeGenerator::ByteCodeGenerator(Js::ScriptContext* scriptContext, Js::Scope flags(0), funcInfoStack(nullptr), jumpCleanupList(nullptr), + nodesToTrackForYield(nullptr), + nodesWithYield(nullptr), pRootFunc(nullptr), pCurrentFunction(nullptr), globalScope(nullptr), @@ -769,6 +783,52 @@ void ByteCodeGenerator::AddFuncInfoToFinalizationSet(FuncInfo * funcInfo) this->funcInfosToFinalize->Prepend(funcInfo); } +void ByteCodeGenerator::PushTrackForYield(ParseNode* node) +{ + if (this->nodesToTrackForYield == nullptr) + { + this->nodesToTrackForYield = Anew(alloc, SList, alloc); + } + this->nodesToTrackForYield->Push(node); +} + +void ByteCodeGenerator::PopTrackForYield(ParseNode* node) +{ + Assert (this->nodesToTrackForYield != nullptr); + Assert (this->nodesToTrackForYield->Top() == node); + this->nodesToTrackForYield->Pop(); + if (this->nodesToTrackForYield->Empty()) + { + this->nodesToTrackForYield = nullptr; + } + else if (this->GetHasYield(node)) + { + this->SetHasYield(); + } +} + +void ByteCodeGenerator::SetHasYield() +{ + if (this->nodesToTrackForYield != nullptr) + { + Assert(!this->nodesToTrackForYield->Empty()); + if (this->nodesWithYield == nullptr) + { + this->nodesWithYield = Anew(alloc, SList, alloc); + } + this->nodesWithYield->Push(this->nodesToTrackForYield->Top()); + } +} + +bool ByteCodeGenerator::GetHasYield(ParseNode* pnode) +{ + if (this->nodesWithYield != nullptr && this->nodesWithYield->Has(pnode)) + { + return true; + } + return false; +} + /* static */ bool ByteCodeGenerator::IsFalse(ParseNode* node) { @@ -2203,6 +2263,8 @@ void ByteCodeGenerator::Begin( this->envDepth = 0; this->trackEnvDepth = false; this->funcInfosToFinalize = nullptr; + this->nodesToTrackForYield = nullptr; + this->nodesWithYield = nullptr; this->funcInfoStack = Anew(alloc, SList, alloc); this->jumpCleanupList = Anew(alloc, JumpCleanupList, alloc); diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.h b/lib/Runtime/ByteCode/ByteCodeGenerator.h index c068ea4089e..bdaf51b100d 100644 --- a/lib/Runtime/ByteCode/ByteCodeGenerator.h +++ b/lib/Runtime/ByteCode/ByteCodeGenerator.h @@ -62,6 +62,8 @@ class ByteCodeGenerator Js::ParseableFunctionInfo * pRootFunc; SList * funcInfosToFinalize; + SList * nodesToTrackForYield; + SList * nodesWithYield; using JumpCleanupList = DList; JumpCleanupList* jumpCleanupList; @@ -485,6 +487,11 @@ class ByteCodeGenerator bool HasJumpCleanup() { return !this->jumpCleanupList->Empty(); } void EmitJumpCleanup(ParseNode* target, FuncInfo* funcInfo); + void ByteCodeGenerator::SetHasYield(); + bool ByteCodeGenerator::GetHasYield(ParseNode* node); + void ByteCodeGenerator::PopTrackForYield(ParseNode* node); + void ByteCodeGenerator::PushTrackForYield(ParseNode* node); + private: bool NeedCheckBlockVar(Symbol* sym, Scope* scope, FuncInfo* funcInfo) const; diff --git a/test/es6GeneratorJit/async-jit-bugs.js b/test/es6GeneratorJit/async-jit-bugs.js index 6b27e1817d8..b114afc8701 100644 --- a/test/es6GeneratorJit/async-jit-bugs.js +++ b/test/es6GeneratorJit/async-jit-bugs.js @@ -4,7 +4,7 @@ // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- -function main() { +function testOne() { const v2 = [13.37,13.37,13.37,13.37,13.37]; async function v4(v5,v6,v7,v8) { const v10 = 0; @@ -26,8 +26,82 @@ function main() { const v38 = --v33; } const v39 = 128; - print("pass") } -v4("vEBD7ei78q"); + return v4("vEBD7ei78q"); } -main(); + +// BugIssue #7034 +function testTwo() { + let finallyCount = 0; + let throwCount = 0; + async function asyncFinally() { + for (let i = 0; i < 1000; ++i){ + try { + if (i > 170) { + ++throwCount; + throw 1; + } + } + finally { + ++finallyCount; + } + } + } + return asyncFinally ().catch((e) => { + if (throwCount != 1) { + throw new Error ("Wrong number of throws within async function expected 1 but received " + throwCount); + } + if (e != 1) { + throw new Error ("Wrong value thrown from async function expected 1 but received " + e); + } + if (finallyCount != 172) { + throw new Error ("Wrong number of finally calls from async function expected 172 but received " + finallyCount); + } + }); +} + +function testThree() { + let finallyCount = 0; + let throwCount = 0; + async function asyncFinallyAwait() { + for (let i = 0; i < 1000; ++i){ + try { + if (i > 170) { + ++throwCount; + throw 1; + } + } + finally { + await 5; + ++finallyCount; + } + } + } + return asyncFinallyAwait().catch((e) => { + if (throwCount != 1) { + throw new Error ("Wrong number of throws within async function expected 1 but received " + throwCount); + } + if (e != 1) { + throw new Error ("Wrong value thrown from async function expected 1 but received " + e); + } + if (finallyCount != 172) { + throw new Error ("Wrong number of finally calls from async function expected 172 but received " + finallyCount); + } + }); +} + +// BugIssue #7016 +function testFour() +{ + async function test() { + var i8 = new Int8Array(256); + var IntArr0 = []; + for (var _strvar1 of i8) { + for (var _strvar1 of IntArr0) {} + } + } + return test(); +} + + +Promise.all([testOne(), testTwo(), testThree(), testFour()]).then(()=>{print("pass")}, (e)=>{print (e)});