Cherry-pick 267815.223@safari-7617-branch (3c47684). https://bugs.web…

…kit.org/show_bug.cgi?id=262222

    BBQJIT if conditions are very wrong
    https://bugs.webkit.org/show_bug.cgi?id=262222
    rdar://problem/116145012

    Reviewed by Keith Miller.

    BBQJIT if conditions are very wrong. By random chance, the condition value
    happens to be allocated in nonPreservedNonArgumentGPR1, but if you use
    more than 8 registers, we end up just reading a completely random value.

    Let's not do that.

    We also add some extra debugging assertions for parallel move. These shouldn't ever actually
    be hit, but they help us avoid a potential problem in the future if we
    make BBQ register allocation smarter.

    Finally, we allow allocating eax on x86, and fix some bugs surrounding if/else as a result.

    * JSTests/wasm/stress/bbq-parallel-move.js: Added.
    (from.string_appeared_here.import.as.assert.from.string_appeared_here.let.wat.module.func.log_value.import.string_appeared_here.string_appeared_here.param.i32.func.export.string_appeared_here.param.p0.i32.param.p1.i32.param.p2.i32.local.p1.local.p1.local.p1.local.p1.local.p1.local.p1.local.p1.local.p1.local.p1.result.i32.local.p0.then.local.p2.local.p0.i32.const.0.else.i32.const.0.local.p2.call.f.func.f.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.pl.i32.call.log_value.local.pl.async test.):
    (from.string_appeared_here.import.as.assert.from.string_appeared_here.let.wat.module.func.log_value.import.string_appeared_here.string_appeared_here.param.i32.func.export.string_appeared_here.param.p0.i32.param.p1.i32.param.p2.i32.local.p1.local.p1.local.p1.local.p1.local.p1.local.p1.local.p1.local.p1.local.p1.result.i32.local.p0.then.local.p2.local.p0.i32.const.0.else.i32.const.0.local.p2.call.f.func.f.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.i32.param.pl.i32.call.log_value.local.pl.async test):
    * Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
    (JSC::Wasm::BBQJIT::ControlData::ControlData):
    (JSC::Wasm::BBQJIT::addIf):
    (JSC::Wasm::BBQJIT::emitIndirectCall):
    (JSC::Wasm::BBQJIT::emitShuffle):

    Canonical link: https://commits.webkit.org/267815.223@safari-7617-branch

Canonical link: https://commits.webkit.org/266719.188@webkitglib/2.42

JSTests/wasm/stress/bbq-parallel-move.js

@@ -0,0 +1,69 @@
+//@ skip if !$isSIMDPlatform
+import { instantiate } from "../wabt-wrapper.js"
+import * as assert from "../assert.js"
+
+let wat = `
+(module
+    (func $log_value (import "a" "log_value") (param i32))
+
+    (func (export "test") (param $p0 i32) (param $p1 i32) (param $p2 i32)
+        (local.get $p1)
+        (local.get $p1)
+        (local.get $p1)
+        (local.get $p1)
+        (local.get $p1)
+        (local.get $p1)
+        (local.get $p1)
+        (local.get $p1)
+        (local.get $p1)
+
+        (if (result i32)
+        (local.get $p0)
+        (then
+            (local.set $p2
+                (local.get $p0))
+            (i32.const 0))
+        (else
+            (i32.const 0)))
+
+        (local.get $p2)
+        (call $f)
+
+    )
+
+    (func $f (param i32) (param i32) (param i32) (param i32) (param i32) (param i32) (param i32) (param i32) (param i32) (param i32) (param $pl i32)
+        (call $log_value
+            (local.get $pl))
+    )
+)
+`
+
+async function test() {
+    let log = []
+    const instance = await instantiate(wat, { a: {
+        log_value: function(i) { log.push("value: " + i); },
+    } }, { simd: true })
+    const { test, test2 } = instance.exports
+
+    let lastLog = []
+    for (let i = 0; i < 10000; ++i) {
+        assert.eq(test(0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21), undefined)
+
+        if (i > 0 && JSON.stringify(lastLog) != JSON.stringify(log)) {
+            print(lastLog)
+            print(log)
+            throw ""
+        }
+
+        lastLog = log
+        log = []
+    }
+
+    print(lastLog)
+    if (lastLog != "value: 3")
+        throw ""
+
+    ;; print("Success")
+}
+
+assert.asyncTest(test())

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

@@ -795,7 +795,7 @@ class BBQJIT {
             : m_enclosedHeight(0)
         { }
 
-        ControlData(BBQJIT& generator, BlockType blockType, BlockSignature signature, LocalOrTempIndex enclosedHeight)
+        ControlData(BBQJIT& generator, BlockType blockType, BlockSignature signature, LocalOrTempIndex enclosedHeight, RegisterSet liveScratchGPRs = { })
             : m_signature(signature)
             , m_blockType(blockType)
             , m_enclosedHeight(enclosedHeight)
@@ -835,8 +835,8 @@ class BBQJIT {
             if (!isAnyCatch(*this)) {
                 auto gprSetCopy = generator.m_validGPRs;
                 auto fprSetCopy = generator.m_validFPRs;
-                // We intentionally exclude GPRInfo::nonPreservedNonArgumentGPR1 from argument locations. See explanation in addIf and emitIndirectCall.
-                gprSetCopy.remove(GPRInfo::nonPreservedNonArgumentGPR1);
+                liveScratchGPRs.forEach([&] (auto r) { gprSetCopy.remove(r); });
+
                 for (unsigned i = 0; i < functionSignature->argumentCount(); ++i)
                     m_argumentLocations.append(allocateArgumentOrResult(generator, functionSignature->argumentType(i).kind, i, gprSetCopy, fprSetCopy));
             }
@@ -847,6 +847,17 @@ class BBQJIT {
                 m_resultLocations.append(allocateArgumentOrResult(generator, functionSignature->returnType(i).kind, i, gprSetCopy, fprSetCopy));
         }
 
+        // Re-use the argument layout of another block (eg. else will re-use the argument/result locations from if)
+        enum BranchCallingConventionReuseTag { UseBlockCallingConventionOfOtherBranch };
+        ControlData(BranchCallingConventionReuseTag, BlockType blockType, ControlData& otherBranch)
+            : m_signature(otherBranch.m_signature)
+            , m_blockType(blockType)
+            , m_argumentLocations(otherBranch.m_argumentLocations)
+            , m_resultLocations(otherBranch.m_resultLocations)
+            , m_enclosedHeight(otherBranch.m_enclosedHeight)
+        {
+        }
+
         template<typename Stack>
         void flushAtBlockBoundary(BBQJIT& generator, unsigned targetArity, Stack& expressionStack, bool endOfWasmBlock)
         {
@@ -7039,7 +7050,10 @@ class BBQJIT {
             emitMove(condition, conditionLocation);
         consume(condition);
 
-        result = ControlData(*this, BlockType::If, signature, currentControlData().enclosedHeight() + currentControlData().implicitSlots() + enclosingStack.size() - signature->as<FunctionSignature>()->argumentCount());
+        RegisterSet liveScratchGPRs;
+        liveScratchGPRs.add(conditionLocation.asGPR(), IgnoreVectors);
+
+        result = ControlData(*this, BlockType::If, signature, currentControlData().enclosedHeight() + currentControlData().implicitSlots() + enclosingStack.size() - signature->as<FunctionSignature>()->argumentCount(), liveScratchGPRs);
 
         // Despite being conditional, if doesn't need to worry about diverging expression stacks at block boundaries, so it doesn't need multiple exits.
         currentControlData().flushAndSingleExit(*this, result, enclosingStack, true, false);
@@ -7052,14 +7066,14 @@ class BBQJIT {
         if (condition.isConst() && !condition.asI32())
             result.setIfBranch(m_jit.jump()); // Emit direct branch if we know the condition is false.
         else if (!condition.isConst()) // Otherwise, we only emit a branch at all if we don't know the condition statically.
-            result.setIfBranch(m_jit.branchTest32(ResultCondition::Zero, GPRInfo::nonPreservedNonArgumentGPR1));
+            result.setIfBranch(m_jit.branchTest32(ResultCondition::Zero, conditionLocation.asGPR()));
         return { };
     }
 
     PartialResult WARN_UNUSED_RETURN addElse(ControlData& data, Stack& expressionStack)
     {
         data.flushAndSingleExit(*this, data, expressionStack, false, true);
-        ControlData dataElse(*this, BlockType::Block, data.signature(), data.enclosedHeight());
+        ControlData dataElse(ControlData::UseBlockCallingConventionOfOtherBranch, BlockType::Block, data);
         data.linkJumps(&m_jit);
         dataElse.addBranch(m_jit.jump());
         data.linkIfBranch(&m_jit); // Link specifically the conditional branch of the preceding If
@@ -7086,7 +7100,7 @@ class BBQJIT {
         // state entering the else block.
         data.flushAtBlockBoundary(*this, 0, m_parser->expressionStack(), true);
 
-        ControlData dataElse(*this, BlockType::Block, data.signature(), data.enclosedHeight());
+        ControlData dataElse(ControlData::UseBlockCallingConventionOfOtherBranch, BlockType::Block, data);
         data.linkJumps(&m_jit);
         dataElse.addBranch(m_jit.jump()); // Still needed even when the parent was unreachable to avoid running code within the else block.
         data.linkIfBranch(&m_jit); // Link specifically the conditional branch of the preceding If
@@ -7936,8 +7950,7 @@ class BBQJIT {
         prepareForExceptions();
         saveValuesAcrossCallAndPassArguments(arguments, wasmCalleeInfo); // Keep in mind that this clobbers wasmScratchGPR and wasmScratchFPR.
 
-        // Why can we still call calleeCode after saveValuesAcrossCallAndPassArguments? This is because we ensured that calleeCode is GPRInfo::nonPreservedNonArgumentGPR1,
-        // and any argument locations will not include GPRInfo::nonPreservedNonArgumentGPR1.
+        // Why can we still call calleeCode after saveValuesAcrossCallAndPassArguments? CalleeCode is a scratch and not any argument GPR.
         m_jit.call(calleeCode, WasmEntryPtrTag);
         returnValuesFromCall(results, *signature.as<FunctionSignature>(), wasmCalleeInfo);
 
@@ -9661,6 +9674,21 @@ class BBQJIT {
     {
         ASSERT(srcVector.size() == dstVector.size());
 
+#if ASSERT_ENABLED
+        for (size_t i = 0; i < dstVector.size(); ++i) {
+            for (size_t j = i + 1; j < dstVector.size(); ++j)
+                ASSERT(dstVector[i] != dstVector[j]);
+        }
+
+        // This algorithm assumes at most one cycle: https://xavierleroy.org/publi/parallel-move.pdf
+        for (size_t i = 0; i < srcVector.size(); ++i) {
+            for (size_t j = i + 1; j < srcVector.size(); ++j) {
+                ASSERT(srcVector[i].isConst() || srcVector[j].isConst()
+                    || locationOf(srcVector[i]) != locationOf(srcVector[j]));
+            }
+        }
+#endif
+
         if (srcVector.size() == 1) {
             emitMove(srcVector[0], dstVector[0]);
             return;