Bug 1347740: Use a rectifier frame when calling from wasm to jit; r=j…

…andem, r=luke

MozReview-Commit-ID: IvkmE7GX8CJ

js/src/jit-test/tests/wasm/import-export.js

@@ -619,26 +619,71 @@ assertEq(e.call(), 1090);
     let valueToConvert = 0;
     function ffi(n) { if (n == 1337) { return valueToConvert }; return 42; }
 
-    // Baseline compile ffi.
-    for (let i = baselineTrigger + 1; i --> 0;)
+    function sum(a, b, c) {
+        if (a === 1337)
+            return valueToConvert;
+        return (a|0) + (b|0) + (c|0) | 0;
+    }
+
+    // Baseline compile ffis.
+    for (let i = baselineTrigger + 1; i --> 0;) {
         ffi(i);
+        sum((i%2)?i:undefined,
+            (i%3)?i:undefined,
+            (i%4)?i:undefined);
+    }
 
-    let imports = { a: { ffi }};
+    let imports = {
+        a: {
+            ffi,
+            sum
+        }
+    };
 
     i = wasmEvalText(`(module
         (import $ffi "a" "ffi" (param i32) (result i32))
-        (func $foo (export "foo") (param i32) (result i32)
+
+        (import $missingOneArg "a" "sum" (param i32) (param i32) (result i32))
+        (import $missingTwoArgs "a" "sum" (param i32) (result i32))
+        (import $missingThreeArgs "a" "sum" (result i32))
+
+        (func (export "foo") (param i32) (result i32)
+         get_local 0
+         call $ffi
+        )
+
+        (func (export "missThree") (result i32)
+         call $missingThreeArgs
+        )
+
+        (func (export "missTwo") (param i32) (result i32)
+         get_local 0
+         call $missingTwoArgs
+        )
+
+        (func (export "missOne") (param i32) (param i32) (result i32)
          get_local 0
-         call $ffi)
+         get_local 1
+         call $missingOneArg
+        )
     )`, imports).exports;
 
-    // Enable the jit exit.
+    // Enable the jit exit for each JS callee.
     assertEq(i.foo(0), 42);
 
-    // Test the jit exit.
+    assertEq(i.missThree(), 0);
+    assertEq(i.missTwo(42), 42);
+    assertEq(i.missOne(13, 37), 50);
+
+    // Test the jit exit under normal conditions.
     assertEq(i.foo(0), 42);
     assertEq(i.foo(1337), 0);
 
+    // Test the arguments rectifier.
+    assertEq(i.missThree(), 0);
+    assertEq(i.missTwo(-1), -1);
+    assertEq(i.missOne(23, 10), 33);
+
     // Test OOL coercion.
     valueToConvert = 2**31;
     assertEq(i.foo(1337), -(2**31));
@@ -649,5 +694,7 @@ assertEq(e.call(), 1090);
 
     valueToConvert = { toString() { throw new Error('a FFI to believe in'); } }
     assertErrorMessage(() => i.foo(1337), Error, "a FFI to believe in");
-})();
 
+    // Test the error path in the arguments rectifier.
+    assertErrorMessage(() => i.missTwo(1337), Error, "a FFI to believe in");
+})();

js/src/jit-test/tests/wasm/profiling.js

@@ -331,9 +331,17 @@ for (let type of ['f32', 'f64']) {
 
     var m = new Module(wasmTextToBinary(`(module
         (import $ffi "a" "ffi" (param i32) (result i32))
-        (func $foo (export "foo") (param i32) (result i32)
+
+        (import $missingOneArg "a" "sumTwo" (param i32) (result i32))
+
+        (func (export "foo") (param i32) (result i32)
          get_local 0
          call $ffi)
+
+        (func (export "id") (param i32) (result i32)
+         get_local 0
+         call $missingOneArg
+        )
     )`));
 
     var valueToConvert = 0;
@@ -343,45 +351,71 @@ for (let type of ['f32', 'f64']) {
         return 42;
     }
 
+    function sumTwo(a, b) {
+        return (a|0)+(b|0)|0;
+    }
+
     // Baseline compile ffi.
-    for (var i = 20; i --> 0;)
+    for (var i = 20; i --> 0;) {
         ffi(i);
+        sumTwo(i-1, i+1);
+    }
 
-    var imports = { a: { ffi }};
+    var imports = {
+        a: {
+            ffi,
+            sumTwo
+        }
+    };
 
     var i = new Instance(m, imports).exports;
 
     // Enable the jit exit.
     assertEq(i.foo(0), 42);
+    assertEq(i.id(13), 13);
 
+    // Test normal conditions.
     enableSingleStepProfiling();
     assertEq(i.foo(0), 42);
-    assertEqStacks(disableSingleStepProfiling(), ["", ">", "1,>", "<,1,>",
+    assertEqStacks(disableSingleStepProfiling(), ["", ">", "2,>", "<,2,>",
         // Losing stack information while the JIT func prologue sets profiler
         // virtual FP.
         "",
         // Callee time.
-        "<,1,>",
+        "<,2,>",
         // Losing stack information while we're exiting JIT func epilogue and
         // recovering wasm FP.
         "",
         // Back into the jit exit (frame info has been recovered).
-        "<,1,>",
+        "<,2,>",
         // Normal unwinding.
-        "1,>", ">", ""]);
+        "2,>", ">", ""]);
+
+    // Test rectifier frame.
+    enableSingleStepProfiling();
+    assertEq(i.id(100), 100);
+    assertEqStacks(disableSingleStepProfiling(), ["", ">", "3,>", "<,3,>",
+        // Rectifier frame time is spent here (lastProfilingFrame has not been
+        // set).
+        "",
+        "<,3,>",
+        // Rectifier frame unwinding time is spent here.
+        "",
+        "<,3,>",
+        "3,>", ">", ""]);
 
     // Test OOL coercion path.
     valueToConvert = 2**31;
 
     enableSingleStepProfiling();
     assertEq(i.foo(1337), -(2**31));
-    assertEqStacks(disableSingleStepProfiling(), ["", ">", "1,>", "<,1,>", "", "<,1,>", "",
+    assertEqStacks(disableSingleStepProfiling(), ["", ">", "2,>", "<,2,>", "", "<,2,>", "",
         // Back into the jit exit (frame info has been recovered).
         // Inline conversion fails, we skip to the OOL path, call from there
         // and get back to the jit exit.
-        "<,1,>",
+        "<,2,>",
         // Normal unwinding.
-        "1,>", ">", ""]);
+        "2,>", ">", ""]);
 
     disableGeckoProfiling();
     setJitCompilerOption("baseline.warmup.trigger", prevOptions["baseline.warmup.trigger"]);

js/src/jit/BaselineBailouts.cpp

@@ -394,11 +394,13 @@ struct BaselineStackBuilder
         BufferPointer<RectifierFrameLayout> priorFrame =
             pointerAtStackOffset<RectifierFrameLayout>(priorOffset);
         FrameType priorType = priorFrame->prevType();
-        MOZ_ASSERT(priorType == JitFrame_IonJS || priorType == JitFrame_BaselineStub);
+        MOZ_ASSERT(priorType == JitFrame_WasmToJSJit ||
+                   priorType == JitFrame_IonJS ||
+                   priorType == JitFrame_BaselineStub);
 
-        // If the frame preceding the rectifier is an IonJS frame, then once again
-        // the frame pointer does not matter.
-        if (priorType == JitFrame_IonJS)
+        // If the frame preceding the rectifier is an IonJS or WasmToJSJit
+        // entry frame, then once again the frame pointer does not matter.
+        if (priorType == JitFrame_IonJS || priorType == JitFrame_WasmToJSJit)
             return nullptr;
 
         // Otherwise, the frame preceding the rectifier is a BaselineStub frame.

js/src/jit/Ion.cpp

@@ -280,6 +280,15 @@ JitRuntime::initialize(JSContext* cx, AutoLockForExclusiveAccess& lock)
             return false;
     }
 
+    // The arguments rectifier has to use the same frame layout as the function
+    // frames it rectifies.
+    static_assert(mozilla::IsBaseOf<JitFrameLayout, RectifierFrameLayout>::value,
+                  "a rectifier frame can be used with jit frame");
+    static_assert(mozilla::IsBaseOf<JitFrameLayout, WasmToJSJitFrameLayout>::value,
+                  "wasm frames simply are jit frames");
+    static_assert(sizeof(JitFrameLayout) == sizeof(WasmToJSJitFrameLayout),
+                  "thus a rectifier frame can be used with a wasm frame");
+
     JitSpew(JitSpew_Codegen, "# Emitting sequential arguments rectifier");
     argumentsRectifier_ = generateArgumentsRectifier(cx, &argumentsRectifierReturnAddr_.writeRef());
     if (!argumentsRectifier_)

js/src/jit/JSJitFrameIter.cpp

@@ -643,6 +643,17 @@ JSJitProfilingFrameIterator::operator++()
     moveToNextFrame(frame);
 }
 
+void
+JSJitProfilingFrameIterator::moveToWasmFrame(CommonFrameLayout* frame)
+{
+    // No previous js jit frame, this is a transition frame, used to
+    // pass a wasm iterator the correct value of FP.
+    returnAddressToFp_ = nullptr;
+    fp_ = GetPreviousRawFrame<uint8_t*>(frame);
+    type_ = JitFrame_WasmToJSJit;
+    MOZ_ASSERT(!done());
+}
+
 void
 JSJitProfilingFrameIterator::moveToNextFrame(CommonFrameLayout* frame)
 {
@@ -666,6 +677,8 @@ JSJitProfilingFrameIterator::moveToNextFrame(CommonFrameLayout* frame)
      * |    ^--- Ion
      * |    |
      * |    ^--- Baseline Stub <---- Baseline
+     * |    |
+     * |    ^--- WasmToJSJit <--- (other wasm frames)
      * |
      * ^--- Entry Frame (From C++)
      *      Exit Frame (From previous JitActivation)
@@ -726,6 +739,11 @@ JSJitProfilingFrameIterator::moveToNextFrame(CommonFrameLayout* frame)
             return;
         }
 
+        if (rectPrevType == JitFrame_WasmToJSJit) {
+            moveToWasmFrame(rectFrame);
+            return;
+        }
+
         MOZ_CRASH("Bad frame type prior to rectifier frame.");
     }
 
@@ -742,11 +760,7 @@ JSJitProfilingFrameIterator::moveToNextFrame(CommonFrameLayout* frame)
     }
 
     if (prevType == JitFrame_WasmToJSJit) {
-        // No previous js jit frame, this is a transition frame, used to pass
-        // a wasm iterator the correct value of FP.
-        returnAddressToFp_ = nullptr;
-        fp_ = GetPreviousRawFrame<uint8_t*>(frame);
-        type_ = JitFrame_WasmToJSJit;
+        moveToWasmFrame(frame);
         return;
     }
 

js/src/jit/JSJitFrameIter.h

@@ -297,6 +297,7 @@ class JSJitProfilingFrameIterator
                                        bool forLastCallSite);
     void fixBaselineReturnAddress();
 
+    void moveToWasmFrame(CommonFrameLayout* frame);
     void moveToNextFrame(CommonFrameLayout* frame);
 
   public:

js/src/jit/JitFrames.h

@@ -445,11 +445,11 @@ class RectifierFrameLayout : public JitFrameLayout
     }
 };
 
-class WasmFrameLayout : public JitFrameLayout
+class WasmToJSJitFrameLayout : public JitFrameLayout
 {
   public:
     static inline size_t Size() {
-        return sizeof(WasmFrameLayout);
+        return sizeof(WasmToJSJitFrameLayout);
     }
 };
 

js/src/jit/MacroAssembler.cpp

@@ -1654,6 +1654,22 @@ MacroAssembler::generateBailoutTail(Register scratch, Register bailoutInfo)
     }
 }
 
+void
+MacroAssembler::assertRectifierFrameParentType(Register frameType)
+{
+#ifdef DEBUG
+    {
+        // Check the possible previous frame types here.
+        Label checkOk;
+        branch32(Assembler::Equal, frameType, Imm32(JitFrame_IonJS), &checkOk);
+        branch32(Assembler::Equal, frameType, Imm32(JitFrame_BaselineStub), &checkOk);
+        branch32(Assembler::Equal, frameType, Imm32(JitFrame_WasmToJSJit), &checkOk);
+        assumeUnreachable("Unrecognized frame type preceding RectifierFrame.");
+        bind(&checkOk);
+    }
+#endif
+}
+
 void
 MacroAssembler::loadBaselineOrIonRaw(Register script, Register dest, Label* failure)
 {

js/src/jit/MacroAssembler.h

@@ -1837,6 +1837,8 @@ class MacroAssembler : public MacroAssemblerSpecific
     // Generates code used to complete a bailout.
     void generateBailoutTail(Register scratch, Register bailoutInfo);
 
+    void assertRectifierFrameParentType(Register frameType);
+
   public:
 #ifndef JS_CODEGEN_ARM64
     // StackPointer manipulation functions.

js/src/jit/arm/Trampoline-arm.cpp

@@ -1287,10 +1287,10 @@ JitRuntime::generateProfilerExitFrameTailStub(JSContext* cx)
     //
     // JitFrame_Rectifier
     //
-    // The rectifier frame can be preceded by either an IonJS or a
-    // BaselineStub frame.
+    // The rectifier frame can be preceded by either an IonJS, a WasmToJSJit or
+    // a BaselineStub frame.
     //
-    // Stack layout if caller of rectifier was Ion:
+    // Stack layout if caller of rectifier was Ion or WasmToJSJit:
     //
     //              Ion-Descriptor
     //              Ion-ReturnAddr
@@ -1335,10 +1335,11 @@ JitRuntime::generateProfilerExitFrameTailStub(JSContext* cx)
         // and |scratch2| points to Rectifier frame
         // and |scratch3| contains Rect-Descriptor.Type
 
+        masm.assertRectifierFrameParentType(scratch3);
+
         // Check for either Ion or BaselineStub frame.
-        Label handle_Rectifier_BaselineStub;
-        masm.branch32(Assembler::NotEqual, scratch3, Imm32(JitFrame_IonJS),
-                      &handle_Rectifier_BaselineStub);
+        Label notIonFrame;
+        masm.branch32(Assembler::NotEqual, scratch3, Imm32(JitFrame_IonJS), &notIonFrame);
 
         // Handle Rectifier <- IonJS
         // scratch3 := RectFrame[ReturnAddr]
@@ -1351,16 +1352,13 @@ JitRuntime::generateProfilerExitFrameTailStub(JSContext* cx)
         masm.storePtr(scratch3, lastProfilingFrame);
         masm.ret();
 
+        masm.bind(&notIonFrame);
+
+        // Check for either BaselineStub or WasmToJSJit: since WasmToJSJit is
+        // just an entry, jump there if we see it.
+        masm.branch32(Assembler::NotEqual, scratch3, Imm32(JitFrame_BaselineStub), &handle_Entry);
+
         // Handle Rectifier <- BaselineStub <- BaselineJS
-        masm.bind(&handle_Rectifier_BaselineStub);
-#ifdef DEBUG
-        {
-            Label checkOk;
-            masm.branch32(Assembler::Equal, scratch3, Imm32(JitFrame_BaselineStub), &checkOk);
-            masm.assumeUnreachable("Unrecognized frame preceding baselineStub.");
-            masm.bind(&checkOk);
-        }
-#endif
         masm.ma_add(scratch2, scratch1, scratch3);
         Address stubFrameReturnAddr(scratch3, RectifierFrameLayout::Size() +
                                               BaselineStubFrameLayout::offsetOfReturnAddress());
@@ -1424,7 +1422,7 @@ JitRuntime::generateProfilerExitFrameTailStub(JSContext* cx)
     }
 
     //
-    // JitFrame_CppToJSJit / JitFrame_WasmJSToJit
+    // JitFrame_CppToJSJit / JitFrame_WasmToJSJit
     //
     // If at an entry frame, store null into both fields.
     // A fast-path wasm->jit transition frame is an entry frame from the point