[wasm][arm64] Fix zero-extention of i32.load8_s

backes · V8 LUCI CQ · commit 748ae7cbf4d7 · 2021-08-13T15:22:23.000Z
InstructionSelector::ZeroExtendsWord32ToWord64 assumes that a Load[kRepWord8|kTypeInt32] generates a zero-extended value. This assumption makes sense, but was not fulfilled by the instruction selector which emitted an "ldrsb" instruction which sign-extended to the full 64-bit register. This CL fixes that by introducing a separate "LdrsbW" instruction which is selected if we are sign-extending an 8-bit value to 32-bit. R=ahaas@chromium.org, mslekova@chromium.org CC=v8-arm-ports@googlegroups.com Bug: chromium:1239116 Change-Id: I2da1ad6062805acf5558f3e66b8db9a50e830302 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3094011 Commit-Queue: Clemens Backes <clemensb@chromium.org> Reviewed-by: Maya Lekova <mslekova@chromium.org> Reviewed-by: Andreas Haas <ahaas@chromium.org> Cr-Commit-Position: refs/heads/master@{#76283}
diff --git a/src/compiler/backend/arm64/code-generator-arm64.cc b/src/compiler/backend/arm64/code-generator-arm64.cc
@@ -104,7 +104,7 @@ class Arm64OperandConverter final : public InstructionOperandConverter {
 
   Register OutputRegister64() { return OutputRegister(); }
 
-  Register OutputRegister32() { return ToRegister(instr_->Output()).W(); }
+  Register OutputRegister32() { return OutputRegister().W(); }
 
   Register TempRegister32(size_t index) {
     return ToRegister(instr_->TempAt(index)).W();
@@ -1763,6 +1763,10 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       EmitOOLTrapIfNeeded(zone(), this, opcode, instr, __ pc_offset());
       __ Ldrsb(i.OutputRegister(), i.MemoryOperand());
       break;
+    case kArm64LdrsbW:
+      EmitOOLTrapIfNeeded(zone(), this, opcode, instr, __ pc_offset());
+      __ Ldrsb(i.OutputRegister32(), i.MemoryOperand());
+      break;
     case kArm64Strb:
       EmitOOLTrapIfNeeded(zone(), this, opcode, instr, __ pc_offset());
       __ Strb(i.InputOrZeroRegister64(0), i.MemoryOperand(1));
diff --git a/src/compiler/backend/arm64/instruction-codes-arm64.h b/src/compiler/backend/arm64/instruction-codes-arm64.h
@@ -164,6 +164,7 @@ namespace compiler {
   V(Arm64StrQ)                              \
   V(Arm64Ldrb)                              \
   V(Arm64Ldrsb)                             \
+  V(Arm64LdrsbW)                            \
   V(Arm64Strb)                              \
   V(Arm64Ldrh)                              \
   V(Arm64Ldrsh)                             \
diff --git a/src/compiler/backend/arm64/instruction-scheduler-arm64.cc b/src/compiler/backend/arm64/instruction-scheduler-arm64.cc
@@ -365,6 +365,7 @@ int InstructionScheduler::GetTargetInstructionFlags(
     case kArm64LdrQ:
     case kArm64Ldrb:
     case kArm64Ldrsb:
+    case kArm64LdrsbW:
     case kArm64Ldrh:
     case kArm64Ldrsh:
     case kArm64Ldrsw:
diff --git a/src/compiler/backend/arm64/instruction-selector-arm64.cc b/src/compiler/backend/arm64/instruction-selector-arm64.cc
@@ -787,7 +787,11 @@ void InstructionSelector::VisitLoad(Node* node) {
       break;
     case MachineRepresentation::kBit:  // Fall through.
     case MachineRepresentation::kWord8:
-      opcode = load_rep.IsSigned() ? kArm64Ldrsb : kArm64Ldrb;
+      opcode = load_rep.IsUnsigned()
+                   ? kArm64Ldrb
+                   : load_rep.semantic() == MachineSemantic::kInt32
+                         ? kArm64LdrsbW
+                         : kArm64Ldrsb;
       immediate_mode = kLoadStoreImm8;
       break;
     case MachineRepresentation::kWord16:
diff --git a/test/unittests/compiler/arm64/instruction-selector-arm64-unittest.cc b/test/unittests/compiler/arm64/instruction-selector-arm64-unittest.cc
@@ -3005,7 +3005,7 @@ std::ostream& operator<<(std::ostream& os, const MemoryAccess& memacc) {
 
 static const MemoryAccess kMemoryAccesses[] = {
     {MachineType::Int8(),
-     kArm64Ldrsb,
+     kArm64LdrsbW,
      kArm64Strb,
      {-256, -255, -3,  -2,   -1,   0,    1,    2,    3,    255,
       256,  257,  258, 1000, 1001, 2121, 2442, 4093, 4094, 4095}},
