Bug 1488515 - Avoid wasm hardfp -> softfp transition twice on Android. r=bbouvier, a=jcristau
authorLars T Hansen <lhansen@mozilla.com>
Wed, 07 Nov 2018 15:12:56 +0100
changeset 498426 2da6ab9b72ac
parent 498425 dc2f3f105676
child 498427 6598cfafc162
push id10154
push userryanvm@gmail.com
push dateFri, 09 Nov 2018 16:38:03 +0000
treeherdermozilla-beta@d84c76f0dddd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbbouvier, jcristau
bugs1488515
milestone64.0
Bug 1488515 - Avoid wasm hardfp -> softfp transition twice on Android. r=bbouvier, a=jcristau The baseline compiler should always call builtins assuming the hardfp calling convention on ARM, but it would actually take the native calling convention on the device into account and sometimes use the softfp convention. The reason the baseline compiler should always use hardfp is that the builtin thunks already convert hardfp->softfp along the call path and softfp->hardfp along the return path, if this is necessary, to allow wasm to call builtins using the hardfp convention always. It is possible that the situation was different when the baseline compiler was written and that the bug is the result of subsequent changes to the thunk layer, but this is not known precisely. There's a driveby fix here to simplify the logic around determining hardfp vs softfp for the system ABI; UseHardFpABI() is now always available and does the right thing, we don't need the #ifdef nest we had previously.
js/src/jit-test/tests/wasm/regress/baseline-builtin-abi.js
js/src/wasm/WasmBaselineCompile.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/wasm/regress/baseline-builtin-abi.js
@@ -0,0 +1,20 @@
+// Bug 1488515 - ABI issues on Android because both the baseline compiler and
+// the builtin thunks converted between hardFP and softFP.
+
+var prog = wasmEvalText(
+    `(module
+       (func $f64div (export "test") (param $a f64) (param $b f64) (result f64)
+         (local $dummy0 i32)
+         (local $dummy1 i32)
+         (local $dummy2 i32)
+         (local $dummy3 i32)
+         (local $dummy4 i32)
+         (local $x f64)
+         (local $y f64)
+         (local $z f64)
+         (set_local $x (get_local $a))
+         (set_local $y (get_local $b))
+         (set_local $z (f64.floor (f64.div (get_local $x) (get_local $y))))
+         (get_local $z)))`);
+
+assertEq(prog.exports.test(16096, 32), 503);
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -162,32 +162,38 @@ using IsKnownNotZero = bool;
 using IsUnsigned = bool;
 using NeedsBoundsCheck = bool;
 using PopStack = bool;
 using WantResult = bool;
 using ZeroOnOverflow = bool;
 
 class BaseStackFrame;
 
+// Two flags, useABI and interModule, control how calls are made.
+//
 // UseABI::Wasm implies that the Tls/Heap/Global registers are nonvolatile,
 // except when InterModule::True is also set, when they are volatile.
 //
+// UseABI::Builtin implies that the Tls/Heap/Global registers are volatile.
+// In this case, we require InterModule::False.  The calling convention
+// is otherwise like UseABI::Wasm.
+//
 // UseABI::System implies that the Tls/Heap/Global registers are volatile.
 // Additionally, the parameter passing mechanism may be slightly different from
 // the UseABI::Wasm convention.
 //
 // When the Tls/Heap/Global registers are not volatile, the baseline compiler
 // will restore the Tls register from its save slot before the call, since the
 // baseline compiler uses the Tls register for other things.
 //
 // When those registers are volatile, the baseline compiler will reload them
 // after the call (it will restore the Tls register from the save slot and load
 // the other two from the Tls data).
 
-enum class UseABI { Wasm, System };
+enum class UseABI { Wasm, Builtin, System };
 enum class InterModule { False = false, True = true };
 
 #if defined(JS_CODEGEN_NONE)
 # define RABALDR_SCRATCH_I32
 # define RABALDR_SCRATCH_F32
 # define RABALDR_SCRATCH_F64
 
 static const Register RabaldrScratchI32 = Register::Invalid();
@@ -3637,33 +3643,34 @@ class BaseCompiler final : public BaseCo
         bool hardFP;
 #endif
         size_t frameAlignAdjustment;
         size_t stackArgAreaSize;
     };
 
     void beginCall(FunctionCall& call, UseABI useABI, InterModule interModule)
     {
+        MOZ_ASSERT_IF(useABI == UseABI::Builtin, interModule == InterModule::False);
+
         call.isInterModule = interModule == InterModule::True;
         call.usesSystemAbi = useABI == UseABI::System;
 
         if (call.usesSystemAbi) {
             // Call-outs need to use the appropriate system ABI.
 #if defined(JS_CODEGEN_ARM)
-# if defined(JS_SIMULATOR_ARM)
             call.hardFP = UseHardFpABI();
-# elif defined(JS_CODEGEN_ARM_HARDFP)
-            call.hardFP = true;
-# else
-            call.hardFP = false;
-# endif
             call.abi.setUseHardFp(call.hardFP);
 #elif defined(JS_CODEGEN_MIPS32)
             call.abi.enforceO32ABI();
 #endif
+        } else {
+#if defined(JS_CODEGEN_ARM)
+            MOZ_ASSERT(call.hardFP,
+                       "All private ABIs pass FP arguments in registers");
+#endif
         }
 
         // Use masm.framePushed() because the value we want here does not depend
         // on the height of the frame's stack area, but the actual size of the
         // allocated frame.
         call.frameAlignAdjustment = ComputeByteAlignment(masm.framePushed() + sizeof(Frame),
                                                          JitStackAlignment);
     }
@@ -8256,17 +8263,17 @@ BaseCompiler::emitUnaryMathBuiltinCall(S
     sync();
 
     ValTypeVector& signature = operandType == ValType::F32 ? SigF_ : SigD_;
     ExprType retType = operandType == ValType::F32 ? ExprType::F32 : ExprType::F64;
     uint32_t numArgs = signature.length();
     size_t stackSpace = stackConsumed(numArgs);
 
     FunctionCall baselineCall(lineOrBytecode);
-    beginCall(baselineCall, UseABI::System, InterModule::False);
+    beginCall(baselineCall, UseABI::Builtin, InterModule::False);
 
     if (!emitCallArgs(signature, &baselineCall)) {
         return false;
     }
 
     builtinCall(callee, baselineCall);
 
     endCall(baselineCall, stackSpace);