Merge pull request #7043 from rhuanjl/fixFinallyinJitGenLoop

Handle Finally Block When Jitting a loop in a Generator or Async function

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;

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);

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<ParseNode*>, 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<ParseNode*>, 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<FuncInfo*>, alloc);
     this->jumpCleanupList = Anew(alloc, JumpCleanupList, alloc);

lib/Runtime/ByteCode/ByteCodeGenerator.h

@@ -62,6 +62,8 @@ private:
     Js::ParseableFunctionInfo * pRootFunc;
 
     SList<FuncInfo*> * funcInfosToFinalize;
+    SList<ParseNode*> * nodesToTrackForYield;
+    SList<ParseNode*> * nodesWithYield;
 
     using JumpCleanupList = DList<JumpCleanupInfo, ArenaAllocator>;
     JumpCleanupList* jumpCleanupList;
@@ -485,6 +487,11 @@ public:
     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;
 

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)});