DirectTailCall implementation needs to tell the shuffler what to put into the Argumen...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Dec 2016 01:25:16 +0000 (01:25 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Dec 2016 01:25:16 +0000 (01:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165882

Reviewed by Mark Lam.
JSTests:

* stress/direct-tail-call-arity-mismatch-count-args.js: Added.
(foo):
(bar):

Source/JavaScriptCore:

The CallFrameShuffler was assuming that the ArgumentCount that it should store into the
callee frame is simply the size of the args vector.

That's not true for DirectTailCall, which will pad the args vector with undefined if we
are optimizing an arity mismatch. We need to pass the ArgumentCount explicitly in this
case.

* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::emitCall):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::emitCall):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileDirectCallOrConstruct):
(JSC::FTL::DFG::LowerDFGToB3::compileTailCall):
* jit/CallFrameShuffleData.h:
* jit/CallFrameShuffler.cpp:
(JSC::CallFrameShuffler::CallFrameShuffler):
(JSC::CallFrameShuffler::prepareAny):
* jit/CallFrameShuffler.h:
(JSC::CallFrameShuffler::snapshot):
* jit/JITCall.cpp:
(JSC::JIT::compileOpCall):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@209846 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/direct-tail-call-arity-mismatch-count-args.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/jit/CallFrameShuffleData.h
Source/JavaScriptCore/jit/CallFrameShuffler.cpp
Source/JavaScriptCore/jit/CallFrameShuffler.h
Source/JavaScriptCore/jit/JITCall.cpp

index bf17037..c0a9342 100644 (file)
@@ -1,3 +1,14 @@
+2016-12-14  Filip Pizlo  <fpizlo@apple.com>
+
+        DirectTailCall implementation needs to tell the shuffler what to put into the ArgumentCount explicitly
+        https://bugs.webkit.org/show_bug.cgi?id=165882
+
+        Reviewed by Mark Lam.
+
+        * stress/direct-tail-call-arity-mismatch-count-args.js: Added.
+        (foo):
+        (bar):
+
 2016-12-14  Keith Miller  <keith_miller@apple.com>
 
         WebAssembly JS API: implement Global
diff --git a/JSTests/stress/direct-tail-call-arity-mismatch-count-args.js b/JSTests/stress/direct-tail-call-arity-mismatch-count-args.js
new file mode 100644 (file)
index 0000000..a2478cf
--- /dev/null
@@ -0,0 +1,20 @@
+"use strict";
+
+function foo(a, b, c, d, e, f) {
+    return arguments.length;
+}
+
+noInline(foo);
+
+function bar() {
+    return foo(1, 2, 3);
+}
+
+noInline(bar);
+
+for (var i = 0; i < 10000; ++i) {
+    var result = bar();
+    if (result != 3)
+        throw "Error: bad result: " + result;
+}
+
index 24ebf47..64612dd 100644 (file)
@@ -1,3 +1,33 @@
+2016-12-14  Filip Pizlo  <fpizlo@apple.com>
+
+        DirectTailCall implementation needs to tell the shuffler what to put into the ArgumentCount explicitly
+        https://bugs.webkit.org/show_bug.cgi?id=165882
+
+        Reviewed by Mark Lam.
+        
+        The CallFrameShuffler was assuming that the ArgumentCount that it should store into the
+        callee frame is simply the size of the args vector.
+        
+        That's not true for DirectTailCall, which will pad the args vector with undefined if we
+        are optimizing an arity mismatch. We need to pass the ArgumentCount explicitly in this
+        case.
+
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::emitCall):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::emitCall):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileDirectCallOrConstruct):
+        (JSC::FTL::DFG::LowerDFGToB3::compileTailCall):
+        * jit/CallFrameShuffleData.h:
+        * jit/CallFrameShuffler.cpp:
+        (JSC::CallFrameShuffler::CallFrameShuffler):
+        (JSC::CallFrameShuffler::prepareAny):
+        * jit/CallFrameShuffler.h:
+        (JSC::CallFrameShuffler::snapshot):
+        * jit/JITCall.cpp:
+        (JSC::JIT::compileOpCall):
+
 2016-12-14  Keith Miller  <keith_miller@apple.com>
 
         WebAssembly JS API: implement Global
index 73fb368..4fa8484 100644 (file)
@@ -879,6 +879,7 @@ void SpeculativeJIT::emitCall(Node* node)
             shuffleData.numLocals = m_jit.graph().frameRegisterCount();
             shuffleData.callee = ValueRecovery::inPair(calleeTagGPR, calleePayloadGPR);
             shuffleData.args.resize(numAllocatedArgs);
+            shuffleData.numPassedArgs = numPassedArgs;
 
             for (unsigned i = 0; i < numPassedArgs; ++i) {
                 Edge argEdge = m_jit.graph().varArgChild(node, i + 1);
index 975be29..e51499d 100644 (file)
@@ -851,7 +851,8 @@ void SpeculativeJIT::emitCall(Node* node)
             shuffleData.numLocals = m_jit.graph().frameRegisterCount();
             shuffleData.callee = ValueRecovery::inGPR(calleeGPR, DataFormatJS);
             shuffleData.args.resize(numAllocatedArgs);
-
+            shuffleData.numPassedArgs = numPassedArgs;
+            
             for (unsigned i = 0; i < numPassedArgs; ++i) {
                 Edge argEdge = m_jit.graph().varArgChild(node, i + 1);
                 GenerationInfo& info = generationInfo(argEdge.node());
index 3cf8d2d..6043063 100644 (file)
@@ -5992,6 +5992,7 @@ private:
                     }
                     for (unsigned i = numPassedArgs; i < numAllocatedArgs; ++i)
                         shuffleData.args.append(ValueRecovery::constant(jsUndefined()));
+                    shuffleData.numPassedArgs = numPassedArgs;
                     shuffleData.setupCalleeSaveRegisters(jit.codeBlock());
                     
                     CallLinkInfo* callLinkInfo = jit.codeBlock()->addCallLinkInfo();
@@ -6158,6 +6159,8 @@ private:
                 for (unsigned i = 0; i < numArgs; ++i)
                     shuffleData.args.append(params[1 + i].recoveryForJSValue());
 
+                shuffleData.numPassedArgs = numArgs;
+                
                 shuffleData.setupCalleeSaveRegisters(jit.codeBlock());
 
                 CallLinkInfo* callLinkInfo = jit.codeBlock()->addCallLinkInfo();
index a987eab..7e3ad5f 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -35,9 +35,10 @@ namespace JSC {
 struct CallFrameShuffleData {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    unsigned numLocals;
+    unsigned numLocals { UINT_MAX };
     ValueRecovery callee;
     Vector<ValueRecovery> args;
+    unsigned numPassedArgs { UINT_MAX };
 #if USE(JSVALUE64)
     RegisterMap<ValueRecovery> registers;
     GPRReg tagTypeNumber { InvalidGPRReg };
index 7209f89..9203e43 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -44,6 +44,7 @@ CallFrameShuffler::CallFrameShuffler(CCallHelpers& jit, const CallFrameShuffleDa
         + roundArgumentCountToAlignFrame(data.args.size()))
     , m_frameDelta(m_alignedNewFrameSize - m_alignedOldFrameSize)
     , m_lockedRegisters(RegisterSet::allRegisters())
+    , m_numPassedArgs(data.numPassedArgs)
 {
     // We are allowed all the usual registers...
     for (unsigned i = GPRInfo::numberOfRegisters; i--; )
@@ -746,7 +747,8 @@ void CallFrameShuffler::prepareAny()
         dataLog("   * Storing the argument count into ", VirtualRegister { CallFrameSlot::argumentCount }, "\n");
     m_jit.store32(MacroAssembler::TrustedImm32(0),
         addressForNew(VirtualRegister { CallFrameSlot::argumentCount }).withOffset(TagOffset));
-    m_jit.store32(MacroAssembler::TrustedImm32(argCount()),
+    RELEASE_ASSERT(m_numPassedArgs != UINT_MAX);
+    m_jit.store32(MacroAssembler::TrustedImm32(m_numPassedArgs),
         addressForNew(VirtualRegister { CallFrameSlot::argumentCount }).withOffset(PayloadOffset));
 
     if (!isSlowPath()) {
index f0918fc..6c0ea33 100644 (file)
@@ -102,6 +102,7 @@ public:
 
         CallFrameShuffleData data;
         data.numLocals = numLocals();
+        data.numPassedArgs = m_numPassedArgs;
         data.callee = getNew(VirtualRegister { CallFrameSlot::callee })->recovery();
         data.args.resize(argCount());
         for (size_t i = 0; i < argCount(); ++i)
@@ -794,6 +795,8 @@ private:
     // It returns false if it was unable to perform some safe writes
     // due to high register pressure.
     bool performSafeWrites();
+    
+    unsigned m_numPassedArgs { UINT_MAX };
 };
 
 } // namespace JSC
index 64ed087..ff1cc61 100644 (file)
@@ -198,6 +198,7 @@ void JIT::compileOpCall(OpcodeID opcodeID, Instruction* instruction, unsigned ca
 
     if (opcodeID == op_tail_call) {
         CallFrameShuffleData shuffleData;
+        shuffleData.numPassedArgs = instruction[3].u.operand;
         shuffleData.tagTypeNumber = GPRInfo::tagTypeNumberRegister;
         shuffleData.numLocals =
             instruction[4].u.operand - sizeof(CallerFrameAndPC) / sizeof(Register);