[compiler] Fix SIMD overlapping issue

The mid-tier register allocator did not handle block merges correctly
where a SIMD register was partially overlapping with a non-SIMD
register. This CL fixes that, and reorders the code to allow for early
exits.

R=mslekova@chromium.org

Bug: chromium:1282224, v8:12330
Change-Id: I2e9275d5c1aaa764ecb63fbf8fa197b68d6b6c3c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3358294
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78482}

src/compiler/backend/mid-tier-register-allocator.cc

@@ -186,6 +186,13 @@ class RegisterIndex final {
     }
   }
 
+  RegisterIndex simdSibling() const {
+    CHECK(!kSimpleFPAliasing);  // Statically evaluated.
+    // Two FP registers {2N, 2N+1} alias the same SIMD register. We compute {2N}
+    // from {2N+1} and vice versa by just flipping the LSB.
+    return RegisterIndex{index_ ^ 1};
+  }
+
   bool operator==(const RegisterIndex& rhs) const {
     return index_ == rhs.index_;
   }
@@ -1759,34 +1766,11 @@ void SinglePassRegisterAllocator::MergeStateFrom(
         if (processed_regs.Contains(reg, rep)) continue;
         processed_regs.Add(reg, rep);
 
-        if (register_state()->IsAllocated(reg)) {
+        bool reg_in_use = register_state()->IsAllocated(reg) ||
-          if (successor_registers->Equals(reg, register_state())) {
+                          (!kSimpleFPAliasing &&
-            // Both match, keep the merged register data.
+                           register_state()->IsAllocated(reg.simdSibling()));
-            register_state()->CommitAtMerge(reg);
-          } else {
-            // Try to find a new register for this successor register in the
-            // merge block, and add a gap move on entry of the successor block.
-            RegisterIndex new_reg =
-                RegisterForVirtualRegister(virtual_register);
-            if (!new_reg.is_valid()) {
-              new_reg = ChooseFreeRegister(
-                  allocated_registers_bits_.Union(succ_allocated_regs), rep);
-            } else if (new_reg != reg) {
-              // Spill the |new_reg| in the successor block to be able to use it
-              // for this gap move. It would be spilled anyway since it contains
-              // a different virtual register than the merge block.
-              SpillRegisterAtMerge(successor_registers, new_reg);
-            }
 
-            if (new_reg.is_valid()) {
+        if (!reg_in_use) {
-              MoveRegisterOnMerge(new_reg, reg, vreg_data, successor,
-                                  successor_registers);
-              processed_regs.Add(new_reg, rep);
-            } else {
-              SpillRegisterAtMerge(successor_registers, reg);
-            }
-          }
-        } else {
           DCHECK(successor_registers->IsAllocated(reg));
           if (RegisterForVirtualRegister(virtual_register).is_valid()) {
             // If we already hold the virtual register in a different register
@@ -1794,12 +1778,40 @@ void SinglePassRegisterAllocator::MergeStateFrom(
             // invalidating the 1:1 vreg<->reg mapping.
             // TODO(rmcilroy): Add a gap move to avoid spilling.
             SpillRegisterAtMerge(successor_registers, reg);
-          } else {
+            continue;
-            // Register is free in our current register state, so merge the
-            // successor block's register details into it.
-            register_state()->CopyFrom(reg, successor_registers);
-            AssignRegister(reg, virtual_register, rep, UsePosition::kNone);
           }
+          // Register is free in our current register state, so merge the
+          // successor block's register details into it.
+          register_state()->CopyFrom(reg, successor_registers);
+          AssignRegister(reg, virtual_register, rep, UsePosition::kNone);
+          continue;
+        }
+
+        // Register is in use in the current register state.
+        if (successor_registers->Equals(reg, register_state())) {
+          // Both match, keep the merged register data.
+          register_state()->CommitAtMerge(reg);
+          continue;
+        }
+        // Try to find a new register for this successor register in the
+        // merge block, and add a gap move on entry of the successor block.
+        RegisterIndex new_reg = RegisterForVirtualRegister(virtual_register);
+        if (!new_reg.is_valid()) {
+          new_reg = ChooseFreeRegister(
+              allocated_registers_bits_.Union(succ_allocated_regs), rep);
+        } else if (new_reg != reg) {
+          // Spill the |new_reg| in the successor block to be able to use it
+          // for this gap move. It would be spilled anyway since it contains
+          // a different virtual register than the merge block.
+          SpillRegisterAtMerge(successor_registers, new_reg);
+        }
+
+        if (new_reg.is_valid()) {
+          MoveRegisterOnMerge(new_reg, reg, vreg_data, successor,
+                              successor_registers);
+          processed_regs.Add(new_reg, rep);
+        } else {
+          SpillRegisterAtMerge(successor_registers, reg);
         }
       }
     }
@@ -2157,10 +2169,7 @@ void SinglePassRegisterAllocator::SpillRegisterAndPotentialSimdSibling(
   SpillRegister(reg);
 
   if (!kSimpleFPAliasing && rep == MachineRepresentation::kSimd128) {
-    // Two FP registers {2N, 2N+1} alias the same SIMD register. We compute {2N}
+    SpillRegister(reg.simdSibling());
-    // from {2N+1} and vice versa in a single computation.
-    RegisterIndex siblingSimdReg{reg.ToInt() + 1 - 2 * (reg.ToInt() & 1)};
-    SpillRegister(siblingSimdReg);
   }
 }
 

test/mjsunit/mjsunit.status

@@ -1464,6 +1464,7 @@
   'regress/wasm/regress-1264462': [SKIP],
   'regress/wasm/regress-1271244': [SKIP],
   'regress/wasm/regress-1271538': [SKIP],
+  'regress/wasm/regress-1282224': [SKIP],
 }],  # no_simd_hardware == True
 
 ##############################################################################

test/mjsunit/regress/wasm/regress-1282224.js

@@ -0,0 +1,31 @@
+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --no-liftoff --turbo-force-mid-tier-regalloc
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+builder.addType(makeSig([kWasmI32, kWasmI32, kWasmI32], [kWasmI32]));
+builder.addFunction(undefined, 0 /* sig */)
+  .addLocals(kWasmS128, 2)
+  .addBody([
+    ...wasmF32Const(0),                       // f32.const
+    ...wasmI32Const(0),                       // f32.const
+    kExprF32SConvertI32,                      // f32.convert_i32_s
+    kExprLocalGet, 3,                         // local.get
+    kSimdPrefix, kExprI64x2AllTrue, 0x01,     // i64x2.all_true
+    kExprSelect,                              // select
+    kExprLocalGet, 4,                         // local.get
+    ...wasmS128Const(new Array(16).fill(0)),  // s128.const
+    kSimdPrefix, kExprI8x16Eq,                // i8x16.eq
+    kSimdPrefix, kExprI64x2AllTrue, 0x01,     // i64x2.all_true
+    kExprF32SConvertI32,                      // f32.convert_i32_s
+    ...wasmS128Const(new Array(16).fill(0)),  // s128.const
+    kSimdPrefix, kExprI64x2AllTrue, 0x01,     // i64x2.all_true
+    kExprSelect,                              // select
+    kExprF32Const, 0x00, 0x00, 0x80, 0x3f,    // f32.const
+    kExprF32Ge,                               // f32.ge
+]);
+builder.toModule();