[PATCH] D85956: [AARCH64][RegisterCoalescer] clang miscompiles zero-extension to long long

[Simon Wallis via Phabricator via llvm-commits]

simonwallis2 created this revision.
simonwallis2 added a reviewer: john.brawn.
Herald added subscribers: llvm-commits, danielkiss, hiraditya, kristof.beyls.
Herald added a project: LLVM.
simonwallis2 requested review of this revision.

Implement AArch64 variant of shouldCoalesce() to detect a known failing case
and prevent the coalescing of a 32-bit copy into a 64-bit sign-extending load.

Do not coalesce in the following case:
COPY where source is bottom 32 bits of a 64-register,
and destination is a 32-bit subregister of a 64-bit register,
ie it causes the rest of the register to be implicitly set to zero.

A mir test has been added.

In the test case, the 32-bit copy implements a 32 to 64 bit zero extension
and relies on the upper 32 bits being zeroed.

Coalescing to the result of the 64-bit load meant overwriting
the upper 32 bits incorrectly when the loaded byte was negative.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85956

Files:
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.h
  llvm/test/CodeGen/AArch64/zext-reg-coalesce.mir


Index: llvm/test/CodeGen/AArch64/zext-reg-coalesce.mir
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/AArch64/zext-reg-coalesce.mir
@@ -0,0 +1,38 @@
+# RUN: llc -mtriple=aarch64-arm-none-eabi -O1 -o - %s \
+# RUN: -run-pass simple-register-coalescing | FileCheck %s
+
+--- |
+  @c = local_unnamed_addr global i8 -1, align 4
+
+  define i64 @bug_e(i32 %i32) local_unnamed_addr {
+  ret i64 0
+  }
+...
+---
+name:            bug_e
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $w0
+
+    %1:gpr32 = COPY $w0
+    %2:gpr64common = ADRP target-flags(aarch64-page) @c
+    %3:gpr64 = LDRSBXui %2, target-flags(aarch64-pageoff, aarch64-nc) @c :: (dereferenceable load 1 from @c, align 4)
+    %0:gpr32 = COPY %3.sub_32
+  ; CHECK: {{.*}}.sub_32:gpr64 = COPY {{.*}}.sub_32
+
+    %4:gpr32 = ANDWrr %0, %1
+    %5:gpr64 = SUBREG_TO_REG 0, killed %4, %subreg.sub_32
+    %6:gpr64 = SUBSXrr %3, killed %5, implicit-def $nzcv
+    Bcc 1, %bb.2, implicit $nzcv
+    B %bb.1
+
+  bb.1:
+    STRBBui %1, %2, target-flags(aarch64-pageoff, aarch64-nc) @c :: (store 1 into @c, align 4)
+
+  bb.2:
+    %8:gpr64all = SUBREG_TO_REG 0, %0, %subreg.sub_32
+    $x0 = COPY %8
+  ; CHECK: $x0 = COPY
+    RET_ReallyLR implicit $x0
+...
Index: llvm/lib/Target/AArch64/AArch64RegisterInfo.h
===================================================================
--- llvm/lib/Target/AArch64/AArch64RegisterInfo.h
+++ llvm/lib/Target/AArch64/AArch64RegisterInfo.h
@@ -125,6 +125,12 @@
 
   unsigned getLocalAddressRegister(const MachineFunction &MF) const;
   bool regNeedsCFI(unsigned Reg, unsigned &RegToUseForCFI) const;
+
+  /// SrcRC and DstRC will be morphed into NewRC if this returns true
+  bool shouldCoalesce(MachineInstr *MI, const TargetRegisterClass *SrcRC,
+                      unsigned SubReg, const TargetRegisterClass *DstRC,
+                      unsigned DstSubReg, const TargetRegisterClass *NewRC,
+                      LiveIntervals &LIS) const override;
 };
 
 } // end namespace llvm
Index: llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
===================================================================
--- llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -726,3 +726,14 @@
     return getBaseRegister();
   return getFrameRegister(MF);
 }
+
+/// SrcRC and DstRC will be morphed into NewRC if this returns true
+bool AArch64RegisterInfo::shouldCoalesce(
+    MachineInstr *MI, const TargetRegisterClass *SrcRC, unsigned SubReg,
+    const TargetRegisterClass *DstRC, unsigned DstSubReg,
+    const TargetRegisterClass *NewRC, LiveIntervals &LIS) const {
+  if (MI->isCopy() && (DstRC->getID() == AArch64::GPR64RegClassID) &&
+      (SrcRC == DstRC) && MI->getOperand(0).getSubReg())
+    return false;
+  return true;
+}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85956.285559.patch
Type: text/x-patch
Size: 2859 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200814/67160aa6/attachment.bin>