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

[Simon Wallis via Phabricator via llvm-commits]

simonwallis2 updated this revision to Diff 288565.
simonwallis2 marked 2 inline comments as done.
simonwallis2 added a comment.

Thanks for the feedback arsenm.
I've made all of the changes you suggested,
with 1 exception:
I have left in the IR for the declaration of @c.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85956/new/

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,33 @@
+# RUN: llc -mtriple=aarch64-arm-none-eabi -o - %s \
+# RUN: -run-pass simple-register-coalescing | FileCheck %s
+
+# In this 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.
+
+--- |
+  @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
+    STRBBui %1, %2, target-flags(aarch64-pageoff, aarch64-nc) @c :: (store 1 into @c, align 4)
+    %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.288565.patch
Type: text/x-patch
Size: 2900 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200828/0ab9e325/attachment-0001.bin>