Bug 1488515 - avoid wasm hardfp -> softfp transition twice on Android. r=bbouvier
authorLars T Hansen <lhansen@mozilla.com>
Wed, 07 Nov 2018 15:12:56 +0100
changeset 501587 f9108997903c74814e3f3b4d8d9548bcbf9e7149
parent 501586 5ece74940a5f39b5c2840400ed05112ebf1eaa15
child 501588 072bf0d7270d25d4892967402c0a1c5eebe0bf9b
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1488515 - avoid wasm hardfp -> softfp transition twice on Android. r=bbouvier 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);