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 501137 2da6ab9b72ac367f2dcf90cceb3dd2f85cbb5aad
parent 501136 dc2f3f105676976a8e48bcce65e2a7c31578f3f6
child 501138 6598cfafc16242e19a616b138af4e142ece943c9
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbbouvier, jcristau
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.
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)
 static const Register RabaldrScratchI32 = Register::Invalid();
@@ -3637,33 +3643,34 @@ class BaseCompiler final : public BaseCo
         bool hardFP;
         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
 #elif defined(JS_CODEGEN_MIPS32)
+        } else {
+#if defined(JS_CODEGEN_ARM)
+            MOZ_ASSERT(call.hardFP,
+                       "All private ABIs pass FP arguments in registers");
         // 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),
@@ -8256,17 +8263,17 @@ BaseCompiler::emitUnaryMathBuiltinCall(S
     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);