[turbofan] Don't assume that Word32AtomicPairLoad has a projection-0

The instruction selector assumed for Word32AtomicPairLoad node that if
there exists a Projection(1) user, then there also exists a
Projection(0) user. This, however, is not the case, because TurboFan
eliminates unreachable nodes. The missing projection node lead to a
failed DCHECK in the register allocator.

To fix the problem I use now the Word32AtomicPairLoad node directly to
allocate the register. On ia32 I stop additionally to allocate unneeded
temp registers.

R=gdeepti@chromium.org
CC=zhin@chromium.org

Bug: chromium:1042379
Change-Id: I79bd9f3f4672e147246a71c32b7c9b4dbd79b17f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2002547
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65912}

src/compiler/backend/arm/code-generator-arm.cc

@@ -3222,10 +3222,27 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       ATOMIC_BINOP_CASE(Xor, eor)
 #undef ATOMIC_BINOP_CASE
     case kArmWord32AtomicPairLoad: {
-      DCHECK(VerifyOutputOfAtomicPairInstr(&i, instr, r0, r1));
-      __ add(i.TempRegister(0), i.InputRegister(0), i.InputRegister(1));
-      __ ldrexd(r0, r1, i.TempRegister(0));
-      __ dmb(ISH);
+      if (instr->OutputCount() == 2) {
+        DCHECK(VerifyOutputOfAtomicPairInstr(&i, instr, r0, r1));
+        __ add(i.TempRegister(0), i.InputRegister(0), i.InputRegister(1));
+        __ ldrexd(r0, r1, i.TempRegister(0));
+        __ dmb(ISH);
+      } else {
+        // A special case of this instruction: even though this is a pair load,
+        // we only need one of the two words. We emit a normal atomic load.
+        DCHECK_EQ(instr->OutputCount(), 1);
+        Register base = i.InputRegister(0);
+        Register offset = i.InputRegister(1);
+        DCHECK(instr->InputAt(2)->IsImmediate());
+        int32_t offset_imm = i.InputInt32(2);
+        if (offset_imm != 0) {
+          Register temp = i.TempRegister(0);
+          __ add(temp, offset, Operand(offset_imm));
+          offset = temp;
+        }
+        __ ldr(i.OutputRegister(), MemOperand(base, offset));
+        __ dmb(ISH);
+      }
       break;
     }
     case kArmWord32AtomicPairStore: {

src/compiler/backend/arm/instruction-selector-arm.cc

@@ -2312,29 +2312,34 @@ void InstructionSelector::VisitWord32AtomicPairLoad(Node* node) {
   ArmOperandGenerator g(this);
   Node* base = node->InputAt(0);
   Node* index = node->InputAt(1);
-  AddressingMode addressing_mode = kMode_Offset_RR;
-  InstructionCode code =
-      kArmWord32AtomicPairLoad | AddressingModeField::encode(addressing_mode);
-  InstructionOperand inputs[] = {g.UseUniqueRegister(base),
-                                 g.UseUniqueRegister(index)};
+  InstructionOperand inputs[3];
+  size_t input_count = 0;
+  inputs[input_count++] = g.UseUniqueRegister(base);
+  inputs[input_count++] = g.UseUniqueRegister(index);
+  InstructionOperand temps[1];
+  size_t temp_count = 0;
+  InstructionOperand outputs[2];
+  size_t output_count = 0;
+
   Node* projection0 = NodeProperties::FindProjection(node, 0);
   Node* projection1 = NodeProperties::FindProjection(node, 1);
-  if (projection1) {
-    InstructionOperand outputs[] = {g.DefineAsFixed(projection0, r0),
-                                    g.DefineAsFixed(projection1, r1)};
-    InstructionOperand temps[] = {g.TempRegister()};
-    Emit(code, arraysize(outputs), outputs, arraysize(inputs), inputs,
-         arraysize(temps), temps);
+  if (projection0 && projection1) {
+    outputs[output_count++] = g.DefineAsFixed(projection0, r0);
+    outputs[output_count++] = g.DefineAsFixed(projection1, r1);
+    temps[temp_count++] = g.TempRegister();
   } else if (projection0) {
-    InstructionOperand outputs[] = {g.DefineAsFixed(projection0, r0)};
-    InstructionOperand temps[] = {g.TempRegister(), g.TempRegister(r1)};
-    Emit(code, arraysize(outputs), outputs, arraysize(inputs), inputs,
-         arraysize(temps), temps);
+    inputs[input_count++] = g.UseImmediate(0);
+    outputs[output_count++] = g.DefineAsRegister(projection0);
+  } else if (projection1) {
+    inputs[input_count++] = g.UseImmediate(4);
+    temps[temp_count++] = g.TempRegister();
+    outputs[output_count++] = g.DefineAsRegister(projection1);
   } else {
-    InstructionOperand temps[] = {g.TempRegister(), g.TempRegister(r0),
-                                  g.TempRegister(r1)};
-    Emit(code, 0, nullptr, arraysize(inputs), inputs, arraysize(temps), temps);
+    // There is no use of the loaded value, we don't need to generate code.
+    return;
   }
+  Emit(kArmWord32AtomicPairLoad, output_count, outputs, input_count, inputs,
+       temp_count, temps);
 }
 
 void InstructionSelector::VisitWord32AtomicPairStore(Node* node) {

src/compiler/backend/ia32/code-generator-ia32.cc

@@ -4102,13 +4102,8 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
     case kIA32Word32AtomicPairLoad: {
       XMMRegister tmp = i.ToDoubleRegister(instr->TempAt(0));
       __ movq(tmp, i.MemoryOperand());
-      if (instr->OutputCount() == 2) {
-        __ Pextrd(i.OutputRegister(0), tmp, 0);
-        __ Pextrd(i.OutputRegister(1), tmp, 1);
-      } else if (instr->OutputCount() == 1) {
-        __ Pextrd(i.OutputRegister(0), tmp, 0);
-        __ Pextrd(i.TempRegister(1), tmp, 1);
-      }
+      __ Pextrd(i.OutputRegister(0), tmp, 0);
+      __ Pextrd(i.OutputRegister(1), tmp, 1);
       break;
     }
     case kIA32Word32AtomicPairStore: {

src/compiler/backend/ia32/instruction-selector-ia32.cc

@@ -87,13 +87,10 @@ class IA32OperandGenerator final : public OperandGenerator {
   }
 
   AddressingMode GenerateMemoryOperandInputs(
-      Node* index, int scale, Node* base, Node* displacement_node,
+      Node* index, int scale, Node* base, int32_t displacement,
       DisplacementMode displacement_mode, InstructionOperand inputs[],
       size_t* input_count, RegisterMode register_mode = kRegister) {
     AddressingMode mode = kMode_MRI;
-    int32_t displacement = (displacement_node == nullptr)
-                               ? 0
-                               : OpParameter<int32_t>(displacement_node->op());
     if (displacement_mode == kNegativeDisplacement) {
       displacement = -displacement;
     }
@@ -148,6 +145,18 @@ class IA32OperandGenerator final : public OperandGenerator {
     return mode;
   }
 
+  AddressingMode GenerateMemoryOperandInputs(
+      Node* index, int scale, Node* base, Node* displacement_node,
+      DisplacementMode displacement_mode, InstructionOperand inputs[],
+      size_t* input_count, RegisterMode register_mode = kRegister) {
+    int32_t displacement = (displacement_node == nullptr)
+                               ? 0
+                               : OpParameter<int32_t>(displacement_node->op());
+    return GenerateMemoryOperandInputs(index, scale, base, displacement,
+                                       displacement_mode, inputs, input_count,
+                                       register_mode);
+  }
+
   AddressingMode GetEffectiveAddressMemoryOperand(
       Node* node, InstructionOperand inputs[], size_t* input_count,
       RegisterMode register_mode = kRegister) {
@@ -1903,28 +1912,33 @@ void InstructionSelector::VisitWord32AtomicPairLoad(Node* node) {
   AddressingMode mode;
   Node* base = node->InputAt(0);
   Node* index = node->InputAt(1);
-  InstructionOperand inputs[] = {g.UseUniqueRegister(base),
-                                 g.GetEffectiveIndexOperand(index, &mode)};
   Node* projection0 = NodeProperties::FindProjection(node, 0);
   Node* projection1 = NodeProperties::FindProjection(node, 1);
-  InstructionCode code =
-      kIA32Word32AtomicPairLoad | AddressingModeField::encode(mode);
-
-  if (projection1) {
+  if (projection0 && projection1) {
+    InstructionOperand inputs[] = {g.UseUniqueRegister(base),
+                                   g.GetEffectiveIndexOperand(index, &mode)};
+    InstructionCode code =
+        kIA32Word32AtomicPairLoad | AddressingModeField::encode(mode);
     InstructionOperand temps[] = {g.TempDoubleRegister()};
     InstructionOperand outputs[] = {g.DefineAsRegister(projection0),
                                     g.DefineAsRegister(projection1)};
-    Emit(code, arraysize(outputs), outputs, arraysize(inputs), inputs,
-         arraysize(temps), temps);
-  } else if (projection0) {
-    InstructionOperand temps[] = {g.TempDoubleRegister(), g.TempRegister()};
-    InstructionOperand outputs[] = {g.DefineAsRegister(projection0)};
-    Emit(code, arraysize(outputs), outputs, arraysize(inputs), inputs,
-         arraysize(temps), temps);
-  } else {
-    InstructionOperand temps[] = {g.TempDoubleRegister(), g.TempRegister(),
-                                  g.TempRegister()};
-    Emit(code, 0, nullptr, arraysize(inputs), inputs, arraysize(temps), temps);
+    Emit(code, 2, outputs, 2, inputs, 1, temps);
+  } else if (projection0 || projection1) {
+    // Only one word is needed, so it's enough to load just that.
+    ArchOpcode opcode = kIA32Movl;
+
+    InstructionOperand outputs[] = {
+        g.DefineAsRegister(projection0 ? projection0 : projection1)};
+    InstructionOperand inputs[3];
+    size_t input_count = 0;
+    // TODO(ahaas): Introduce an enum for {scale} instead of an integer.
+    // {scale = 0} means *1 in the generated code.
+    int scale = 0;
+    AddressingMode mode = g.GenerateMemoryOperandInputs(
+        index, scale, base, projection0 ? 0 : 4, kPositiveDisplacement, inputs,
+        &input_count);
+    InstructionCode code = opcode | AddressingModeField::encode(mode);
+    Emit(code, 1, outputs, input_count, inputs);
   }
 }
 

test/cctest/wasm/test-run-wasm-atomics64.cc

@@ -694,6 +694,35 @@ WASM_EXEC_TEST(AtomicCompareExchangeNoConsideredEffectful) {
   CHECK_EQ(1, r.Call());
 }
 
+WASM_EXEC_TEST(I64AtomicLoadUseOnlyLowWord) {
+  EXPERIMENTAL_FLAG_SCOPE(threads);
+  WasmRunner<uint32_t> r(execution_tier);
+  uint64_t* memory =
+      r.builder().AddMemoryElems<uint64_t>(kWasmPageSize / sizeof(uint64_t));
+  memory[1] = 0x1234567890abcdeful;
+  r.builder().SetHasSharedMemory();
+  // Test that we can use just the low word of an I64AtomicLoad.
+  BUILD(r,
+        WASM_I32_CONVERT_I64(WASM_ATOMICS_LOAD_OP(
+            kExprI64AtomicLoad, WASM_I32V(8), MachineRepresentation::kWord64)));
+  CHECK_EQ(0x90abcdef, r.Call());
+}
+
+WASM_EXEC_TEST(I64AtomicLoadUseOnlyHighWord) {
+  EXPERIMENTAL_FLAG_SCOPE(threads);
+  WasmRunner<uint32_t> r(execution_tier);
+  uint64_t* memory =
+      r.builder().AddMemoryElems<uint64_t>(kWasmPageSize / sizeof(uint64_t));
+  memory[1] = 0x1234567890abcdeful;
+  r.builder().SetHasSharedMemory();
+  // Test that we can use just the high word of an I64AtomicLoad.
+  BUILD(r, WASM_I32_CONVERT_I64(WASM_I64_ROR(
+               WASM_ATOMICS_LOAD_OP(kExprI64AtomicLoad, WASM_I32V(8),
+                                    MachineRepresentation::kWord64),
+               WASM_I64V(32))));
+  CHECK_EQ(0x12345678, r.Call());
+}
+
 }  // namespace test_run_wasm_atomics_64
 }  // namespace wasm
 }  // namespace internal