Bladeren bron

[MERGE #6247 @zenparsing] Insert a ByteCodeUses instr for arguments when inlining apply target

Merge pull request #6247 from zenparsing:inline-apply-target-bug

When inline apply target optimization is used for:

```js
inlinee.apply(obj, arguments);
```

a bailout will occur if the target function is not the expected inlinee. When this occurs, the arguments object needs to be created for use in the interpreter. Currently, GlobOpt is not correctly tracking that the arguments object symbol is needed by the interpreter, and the arguments object is not getting added to the associated bailout info.

This PR adds a `ByteCodeUses` instr for this inlining case.

Additional changes:

- Explicit initialization for the `m_nonEscapingArgObjAlias` field of StackSym.
- `Src2` is no longer used for `BytecodeArgOutUse` (GlobOpt was not checking Src2)
Kevin Smith 6 jaren geleden
bovenliggende
commit
8fcb0f1b57
4 gewijzigde bestanden met toevoegingen van 43 en 2 verwijderingen
  1. 10 2
      lib/Backend/Inline.cpp
  2. 1 0
      lib/Backend/Sym.cpp
  3. 27 0
      test/inlining/applyBailoutArgs.js
  4. 5 0
      test/inlining/rlexe.xml

+ 10 - 2
lib/Backend/Inline.cpp

@@ -3088,15 +3088,23 @@ bool Inline::InlineApplyScriptTarget(IR::Instr *callInstr, const FunctionJITTime
         // set src1 to avoid CSE on BailOnNotStackArgs for different arguments object
         bailOutOnNotStackArgs->SetSrc1(argumentsObjArgOut->GetSrc1()->Copy(this->topFunc));
         argumentsObjArgOut->InsertBefore(bailOutOnNotStackArgs);
+
+        // Insert ByteCodeUses instr to ensure that arguments object is available on bailout
+        IR::ByteCodeUsesInstr* bytecodeUses = IR::ByteCodeUsesInstr::New(callInstr);
+        IR::Opnd* argSrc1 = argObjByteCodeArgoutCapture->GetSrc1();
+        bytecodeUses->SetRemovedOpndSymbol(argSrc1->GetIsJITOptimizedReg(), argSrc1->GetStackSym()->m_id);
+        callInstr->InsertBefore(bytecodeUses);
     }
 
     IR::Instr* byteCodeArgOutUse = IR::Instr::New(Js::OpCode::BytecodeArgOutUse, callInstr->m_func);
     byteCodeArgOutUse->SetSrc1(implicitThisArgOut->GetSrc1());
+    callInstr->InsertBefore(byteCodeArgOutUse);
     if (argumentsObjArgOut)
     {
-        byteCodeArgOutUse->SetSrc2(argumentsObjArgOut->GetSrc1());
+        byteCodeArgOutUse = IR::Instr::New(Js::OpCode::BytecodeArgOutUse, callInstr->m_func);
+        byteCodeArgOutUse->SetSrc1(argumentsObjArgOut->GetSrc1());
+        callInstr->InsertBefore(byteCodeArgOutUse);
     }
-    callInstr->InsertBefore(byteCodeArgOutUse);
 
     // don't need the implicit "this" anymore
     explicitThisArgOut->ReplaceSrc2(startCall->GetDst());

+ 1 - 0
lib/Backend/Sym.cpp

@@ -48,6 +48,7 @@ StackSym::New(SymID id, IRType type, Js::RegSlot byteCodeRegSlot, Func *func)
     stackSym->m_isTypeSpec = false;
     stackSym->m_isArgSlotSym = false;
     stackSym->m_isArgSlotRegSym = false;
+    stackSym->m_nonEscapingArgObjAlias = false;
     stackSym->m_isParamSym = false;
     stackSym->m_isImplicitParamSym = false;
     stackSym->m_isBailOutReferenced = false;

+ 27 - 0
test/inlining/applyBailoutArgs.js

@@ -0,0 +1,27 @@
+var hasArgs = true;
+
+function method0() {}
+
+function f1(a = (hasArgs = false)) {}
+  
+function f2() {
+  this.method0.apply(this, arguments);
+}
+
+function test0() {
+  var obj1 = {};
+  obj1.method0 = f1;
+  obj1.method1 = f2;
+  obj1.method1.call(undefined);
+  obj1.method1(1);
+}
+
+test0();
+test0();
+test0();
+
+if (hasArgs) {
+  WScript.Echo('Passed');
+} else {
+  WScript.Echo('Arguments not passed to inlinee on bailout');
+}

+ 5 - 0
test/inlining/rlexe.xml

@@ -107,6 +107,11 @@
       <compile-flags>-maxinterpretcount:1 -maxsimplejitruncount:0</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>applyBailoutArgs.js</files>
+    </default>
+  </test>
   <test>
     <default>
       <files>bugs.js</files>