Przeglądaj źródła

[MERGE #5250 @Cellule] Post-Op bailout with unconditional branches

Merge pull request #5250 from Cellule:post_op_bailout

When searching for new bytecode instr for a post-op bailout, follow unconditional branches.

OS#17449647
Michael Ferris 7 lat temu
rodzic
commit
dfd11cd34b

+ 3 - 1
lib/Backend/FlowGraph.cpp

@@ -317,7 +317,9 @@ FlowGraph::Build(void)
 
             Assert(leaveTarget->labelRefs.HasOne());
             IR::BranchInstr * brOnException = IR::BranchInstr::New(Js::OpCode::BrOnException, finallyLabel, instr->m_func);
-            leaveTarget->labelRefs.Head()->InsertBefore(brOnException);
+            IR::BranchInstr * leaveInstr = leaveTarget->labelRefs.Head();
+            brOnException->SetByteCodeOffset(leaveInstr);
+            leaveInstr->InsertBefore(brOnException);
 
             instrPrev = instr->m_prev;
         }

+ 2 - 16
lib/Backend/GlobOptBailOut.cpp

@@ -1372,24 +1372,10 @@ GlobOpt::MayNeedBailOnImplicitCall(IR::Instr const * instr, Value const * src1Va
 void
 GlobOpt::GenerateBailAfterOperation(IR::Instr * *const pInstr, IR::BailOutKind kind)
 {
-    Assert(pInstr);
+    Assert(pInstr && *pInstr);
 
     IR::Instr* instr = *pInstr;
-    Assert(instr);
-
-    IR::Instr * nextInstr = instr->GetNextRealInstrOrLabel();
-    uint32 currentOffset = instr->GetByteCodeOffset();
-    while (nextInstr->GetByteCodeOffset() == Js::Constants::NoByteCodeOffset ||
-        nextInstr->GetByteCodeOffset() == currentOffset)
-    {
-        nextInstr = nextInstr->GetNextRealInstrOrLabel();
-    }
-    // This can happen due to break block removal
-    while (nextInstr->GetByteCodeOffset() == Js::Constants::NoByteCodeOffset ||
-        nextInstr->GetByteCodeOffset() < currentOffset)
-    {
-        nextInstr = nextInstr->GetNextRealInstrOrLabel();
-    }
+    IR::Instr * nextInstr = instr->GetNextByteCodeInstr();
     IR::Instr * bailOutInstr = instr->ConvertToBailOutInstr(nextInstr, kind);
     if (this->currentBlock->GetLastInstr() == instr)
     {

+ 32 - 0
lib/Backend/IR.cpp

@@ -2704,6 +2704,38 @@ Instr::GetNextBranchOrLabel() const
     return instr;
 }
 
+IR::Instr *
+Instr::GetNextByteCodeInstr() const
+{
+    IR::Instr * nextInstr = GetNextRealInstrOrLabel();
+    uint32 currentOffset = GetByteCodeOffset();
+    const auto getNext = [](IR::Instr* nextInstr) -> IR::Instr*
+    {
+        if (nextInstr->IsBranchInstr())
+        {
+            IR::BranchInstr* branchInstr = nextInstr->AsBranchInstr();
+            AssertMsg(branchInstr->IsUnconditional(), "We can't know which branch to take on a conditionnal branch");
+            if (branchInstr->IsUnconditional())
+            {
+                return branchInstr->GetTarget();
+            }
+        }
+        return nextInstr->GetNextRealInstrOrLabel();
+    };
+    while (nextInstr->GetByteCodeOffset() == Js::Constants::NoByteCodeOffset ||
+        nextInstr->GetByteCodeOffset() == currentOffset)
+    {
+        nextInstr = getNext(nextInstr);
+    }
+    // This can happen due to break block removal
+    while (nextInstr->GetByteCodeOffset() == Js::Constants::NoByteCodeOffset ||
+        nextInstr->GetByteCodeOffset() < currentOffset)
+    {
+        nextInstr = getNext(nextInstr);
+    }
+    return nextInstr;
+}
+
 ///----------------------------------------------------------------------------
 ///
 /// Instr::GetPrevRealInstr

+ 1 - 0
lib/Backend/IR.h

@@ -288,6 +288,7 @@ public:
     IR::Instr *     GetNextRealInstr() const;
     IR::Instr *     GetNextRealInstrOrLabel() const;
     IR::Instr *     GetNextBranchOrLabel() const;
+    IR::Instr *     GetNextByteCodeInstr() const;
     IR::Instr *     GetPrevRealInstr() const;
     IR::Instr *     GetPrevRealInstrOrLabel() const;
     IR::LabelInstr *GetPrevLabelInstr() const;

+ 17 - 0
test/bailout/bug17449647.js

@@ -0,0 +1,17 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+var strvar1 = "";
+function foo() {
+  switch (strvar1) {
+  case 1:
+    this();
+  case "":
+  }
+}
+for (let i = 0; i < 1000; ++i) {
+  foo();
+}
+console.log("pass");

+ 5 - 0
test/bailout/rlexe.xml

@@ -270,4 +270,9 @@
       <files>bug13674598.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>bug17449647.js</files>
+    </default>
+  </test>
 </regress-exe>