]> granicus.if.org Git - llvm/commitdiff
[AVR] Fix codegen bug in 16-bit loads
authorDylan McKay <me@dylanmckay.io>
Sun, 20 Jan 2019 03:41:08 +0000 (03:41 +0000)
committerDylan McKay <me@dylanmckay.io>
Sun, 20 Jan 2019 03:41:08 +0000 (03:41 +0000)
Prior to this patch, the AVR::LDWRdPtr instruction was always lowered to
instructions of this pattern:

    ld  $GPR8, [PTR:XYZ]+
    ld  $GPR8, [PTR]+1

This has a problem; the [PTR] is incremented in-place once, but never
decremented.

Future uses of the same pointer will use the now clobbered value,
leading to the pointer being incorrect by an offset of one.

This patch modifies the expansion code of the LDWRdPtr pseudo
instruction so that the pointer variable is not silently clobbered in
future uses in the same live range.

Bug first reported by Keshav Kini.

Patch by Kaushik Phatak.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@351673 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/AVR/AVRExpandPseudoInsts.cpp
lib/Target/AVR/AVRISelLowering.cpp
test/CodeGen/AVR/PR37143.ll [new file with mode: 0644]
test/CodeGen/AVR/atomics/load16.ll
test/CodeGen/AVR/load.ll
test/CodeGen/AVR/pseudo/LDWRdPtr-same-src-dst.mir
test/CodeGen/AVR/pseudo/LDWRdPtr.mir

index 60924c2390190b2da75e93bac3e69a9294c603d1..c45b2d0e39c1a4f8771e75825ace2bbf4015a976 100644 (file)
@@ -582,8 +582,8 @@ bool AVRExpandPseudo::expand<AVR::LDWRdPtr>(Block &MBB, BlockIt MBBI) {
   unsigned TmpReg = 0; // 0 for no temporary register
   unsigned SrcReg = MI.getOperand(1).getReg();
   bool SrcIsKill = MI.getOperand(1).isKill();
-  OpLo = AVR::LDRdPtrPi;
-  OpHi = AVR::LDRdPtr;
+  OpLo = AVR::LDRdPtr;
+  OpHi = AVR::LDDRdPtrQ;
   TRI->splitReg(DstReg, DstLoReg, DstHiReg);
 
   // Use a temporary register if src and dst registers are the same.
@@ -596,8 +596,7 @@ bool AVRExpandPseudo::expand<AVR::LDWRdPtr>(Block &MBB, BlockIt MBBI) {
   // Load low byte.
   auto MIBLO = buildMI(MBB, MBBI, OpLo)
     .addReg(CurDstLoReg, RegState::Define)
-    .addReg(SrcReg, RegState::Define)
-    .addReg(SrcReg);
+    .addReg(SrcReg, RegState::Define);
 
   // Push low byte onto stack if necessary.
   if (TmpReg)
@@ -606,7 +605,8 @@ bool AVRExpandPseudo::expand<AVR::LDWRdPtr>(Block &MBB, BlockIt MBBI) {
   // Load high byte.
   auto MIBHI = buildMI(MBB, MBBI, OpHi)
     .addReg(CurDstHiReg, RegState::Define)
-    .addReg(SrcReg, getKillRegState(SrcIsKill));
+    .addReg(SrcReg, getKillRegState(SrcIsKill))
+    .addImm(1);
 
   if (TmpReg) {
     // Move the high byte into the final destination.
index 42a6b46ed7f82a537a1d619057ac51d32201ed69..3116eb2a50321e45b365cc18fbf7eafdd9487a23 100644 (file)
@@ -1278,7 +1278,6 @@ SDValue AVRTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
   }
 
   // Add a register mask operand representing the call-preserved registers.
-  const AVRTargetMachine &TM = (const AVRTargetMachine &)getTargetMachine();
   const TargetRegisterInfo *TRI = Subtarget.getRegisterInfo();
   const uint32_t *Mask =
       TRI->getCallPreservedMask(DAG.getMachineFunction(), CallConv);
@@ -1441,7 +1440,6 @@ MachineBasicBlock *AVRTargetLowering::insertShift(MachineInstr &MI,
   bool HasRepeatedOperand = false;
   MachineFunction *F = BB->getParent();
   MachineRegisterInfo &RI = F->getRegInfo();
-  const AVRTargetMachine &TM = (const AVRTargetMachine &)getTargetMachine();
   const TargetInstrInfo &TII = *Subtarget.getInstrInfo();
   DebugLoc dl = MI.getDebugLoc();
 
@@ -1582,7 +1580,6 @@ static bool isCopyMulResult(MachineBasicBlock::iterator const &I) {
 // it, but it works for now.
 MachineBasicBlock *AVRTargetLowering::insertMul(MachineInstr &MI,
                                                 MachineBasicBlock *BB) const {
-  const AVRTargetMachine &TM = (const AVRTargetMachine &)getTargetMachine();
   const TargetInstrInfo &TII = *Subtarget.getInstrInfo();
   MachineBasicBlock::iterator I(MI);
   ++I; // in any case insert *after* the mul instruction
diff --git a/test/CodeGen/AVR/PR37143.ll b/test/CodeGen/AVR/PR37143.ll
new file mode 100644 (file)
index 0000000..db157ed
--- /dev/null
@@ -0,0 +1,13 @@
+; RUN: llc -mattr=avr6,sram < %s -march=avr | FileCheck %s
+
+; CHECK: ld {{r[0-9]+}}, [[PTR:[YZ]]]
+; CHECK: ldd {{r[0-9]+}}, [[PTR]]+1
+; CHECK: st [[PTR]], {{r[0-9]+}}
+; CHECK: std [[PTR]]+1, {{r[0-9]+}}
+define void @load_store_16(i16* nocapture %ptr) local_unnamed_addr #1 {
+entry:
+  %0 = load i16, i16* %ptr, align 2
+  %add = add i16 %0, 5
+  store i16 %add, i16* %ptr, align 2
+  ret void
+}
index 2b51afe45f4fa34c0a32af28fd60bcb14232781b..6b96edbdab8e7cf5369b70b21713bd481c3937b2 100644 (file)
@@ -3,8 +3,8 @@
 ; CHECK-LABEL: atomic_load16
 ; CHECK:      in r0, 63
 ; CHECK-NEXT: cli
-; CHECK-NEXT: ld [[RR:r[0-9]+]], [[RD:(X|Y|Z)]]+
-; CHECK-NEXT: ld [[RR:r[0-9]+]], [[RD:(X|Y|Z)]]
+; CHECK-NEXT: ld  [[RR:r[0-9]+]], [[RD:(X|Y|Z)]]
+; CHECK-NEXT: ldd [[RR:r[0-9]+]], [[RD]]+1
 ; CHECK-NEXT: out 63, r0
 define i16 @atomic_load16(i16* %foo) {
   %val = load atomic i16, i16* %foo unordered, align 2
@@ -29,12 +29,12 @@ define i16 @atomic_load_cmp_swap16(i16* %foo) {
 ; CHECK-LABEL: atomic_load_add16
 ; CHECK:      in r0, 63
 ; CHECK-NEXT: cli
-; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD1:(X|Y|Z)]]+
-; CHECK-NEXT: ld [[RR2:r[0-9]+]], [[RD2:(X|Y|Z)]]
+; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD:(X|Y|Z)]]
+; CHECK-NEXT: ldd [[RR2:r[0-9]+]], [[RD]]+1
 ; CHECK-NEXT: add [[RR1]], [[TMP:r[0-9]+]]
 ; CHECK-NEXT: adc [[RR2]], [[TMP:r[0-9]+]]
-; CHECK-NEXT: st [[RD1]], [[RR1]]
-; CHECK-NEXT: std [[RD1]]+1, [[A:r[0-9]+]]
+; CHECK-NEXT: st [[RD]], [[RR1]]
+; CHECK-NEXT: std [[RD]]+1, [[A:r[0-9]+]]
 ; CHECK-NEXT: out 63, r0
 define i16 @atomic_load_add16(i16* %foo) {
   %val = atomicrmw add i16* %foo, i16 13 seq_cst
@@ -44,12 +44,12 @@ define i16 @atomic_load_add16(i16* %foo) {
 ; CHECK-LABEL: atomic_load_sub16
 ; CHECK:      in r0, 63
 ; CHECK-NEXT: cli
-; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD1:(X|Y|Z)]]+
-; CHECK-NEXT: ld [[RR2:r[0-9]+]], [[RD2:(X|Y|Z)]]
+; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD:(X|Y|Z)]]
+; CHECK-NEXT: ldd [[RR2:r[0-9]+]], [[RD]]+1
 ; CHECK-NEXT: sub [[RR1]], [[TMP:r[0-9]+]]
 ; CHECK-NEXT: sbc [[RR2]], [[TMP:r[0-9]+]]
-; CHECK-NEXT: st [[RD1]], [[RR1]]
-; CHECK-NEXT: std [[RD1]]+1, [[A:r[0-9]+]]
+; CHECK-NEXT: st [[RD]], [[RR1]]
+; CHECK-NEXT: std [[RD]]+1, [[A:r[0-9]+]]
 ; CHECK-NEXT: out 63, r0
 define i16 @atomic_load_sub16(i16* %foo) {
   %val = atomicrmw sub i16* %foo, i16 13 seq_cst
@@ -59,12 +59,12 @@ define i16 @atomic_load_sub16(i16* %foo) {
 ; CHECK-LABEL: atomic_load_and16
 ; CHECK:      in r0, 63
 ; CHECK-NEXT: cli
-; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD1:(X|Y|Z)]]+
-; CHECK-NEXT: ld [[RR2:r[0-9]+]], [[RD2:(X|Y|Z)]]
+; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD:(X|Y|Z)]]
+; CHECK-NEXT: ldd [[RR2:r[0-9]+]], [[RD]]+1
 ; CHECK-NEXT: and [[RR1]], [[TMP:r[0-9]+]]
 ; CHECK-NEXT: and [[RR2]], [[TMP:r[0-9]+]]
-; CHECK-NEXT: st [[RD1]], [[RR1]]
-; CHECK-NEXT: std [[RD1]]+1, [[A:r[0-9]+]]
+; CHECK-NEXT: st [[RD]], [[RR1]]
+; CHECK-NEXT: std [[RD]]+1, [[A:r[0-9]+]]
 ; CHECK-NEXT: out 63, r0
 define i16 @atomic_load_and16(i16* %foo) {
   %val = atomicrmw and i16* %foo, i16 13 seq_cst
@@ -74,12 +74,12 @@ define i16 @atomic_load_and16(i16* %foo) {
 ; CHECK-LABEL: atomic_load_or16
 ; CHECK:      in r0, 63
 ; CHECK-NEXT: cli
-; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD1:(X|Y|Z)]]+
-; CHECK-NEXT: ld [[RR2:r[0-9]+]], [[RD2:(X|Y|Z)]]
+; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD:(X|Y|Z)]]
+; CHECK-NEXT: ldd [[RR2:r[0-9]+]], [[RD]]+1
 ; CHECK-NEXT: or [[RR1]], [[TMP:r[0-9]+]]
 ; CHECK-NEXT: or [[RR2]], [[TMP:r[0-9]+]]
-; CHECK-NEXT: st [[RD1]], [[RR1]]
-; CHECK-NEXT: std [[RD1]]+1, [[A:r[0-9]+]]
+; CHECK-NEXT: st [[RD]], [[RR1]]
+; CHECK-NEXT: std [[RD]]+1, [[A:r[0-9]+]]
 ; CHECK-NEXT: out 63, r0
 define i16 @atomic_load_or16(i16* %foo) {
   %val = atomicrmw or i16* %foo, i16 13 seq_cst
@@ -89,12 +89,12 @@ define i16 @atomic_load_or16(i16* %foo) {
 ; CHECK-LABEL: atomic_load_xor16
 ; CHECK:      in r0, 63
 ; CHECK-NEXT: cli
-; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD1:(X|Y|Z)]]+
-; CHECK-NEXT: ld [[RR2:r[0-9]+]], [[RD2:(X|Y|Z)]]
+; CHECK-NEXT: ld [[RR1:r[0-9]+]], [[RD:(X|Y|Z)]]
+; CHECK-NEXT: ldd [[RR2:r[0-9]+]], [[RD]]+1
 ; CHECK-NEXT: eor [[RR1]], [[TMP:r[0-9]+]]
 ; CHECK-NEXT: eor [[RR2]], [[TMP:r[0-9]+]]
-; CHECK-NEXT: st [[RD1]], [[RR1]]
-; CHECK-NEXT: std [[RD1]]+1, [[A:r[0-9]+]]
+; CHECK-NEXT: st [[RD]], [[RR1]]
+; CHECK-NEXT: std [[RD]]+1, [[A:r[0-9]+]]
 ; CHECK-NEXT: out 63, r0
 define i16 @atomic_load_xor16(i16* %foo) {
   %val = atomicrmw xor i16* %foo, i16 13 seq_cst
index 73568b54096566cb884a25a1d01e75b61e4d9112..f58edeb425a72189f07ba42249a43ea8dd3d85e6 100644 (file)
@@ -9,8 +9,8 @@ define i8 @load8(i8* %x) {
 
 define i16 @load16(i16* %x) {
 ; CHECK-LABEL: load16:
-; CHECK: ld r24, {{[XYZ]}}+
-; CHECK: ld r25, {{[XYZ]}}
+; CHECK: ld r24,  [[PTR:[XYZ]]]
+; CHECK: ldd r25, [[PTR]]+1
   %1 = load i16, i16* %x
   ret i16 %1
 }
@@ -36,8 +36,8 @@ define i8 @load8nodisp(i8* %x) {
 
 define i16 @load16disp(i16* %x) {
 ; CHECK-LABEL: load16disp:
-; CHECK: ldd r24, {{[YZ]}}+62
-; CHECK: ldd r25, {{[YZ]}}+63
+; CHECK: ldd r24, [[PTR:[YZ]]]+62
+; CHECK: ldd r25, [[PTR]]+63
   %1 = getelementptr inbounds i16, i16* %x, i64 31
   %2 = load i16, i16* %1
   ret i16 %2
@@ -48,8 +48,8 @@ define i16 @load16nodisp(i16* %x) {
 ; CHECK: movw r26, r24
 ; CHECK: subi r26, 192
 ; CHECK: sbci r27, 255
-; CHECK: ld r24, {{[XYZ]}}+
-; CHECK: ld r25, {{[XYZ]}}
+; CHECK: ld r24,  [[PTR:[XYZ]]]
+; CHECK: ldd r25, [[PTR]]+1
   %1 = getelementptr inbounds i16, i16* %x, i64 32
   %2 = load i16, i16* %1
   ret i16 %2
index 4fc97f63d95c47cf9ea370197f5fafe4000ebc28..3c3a7219ee3e403d08dcb12d548e4e93ce23e082 100644 (file)
@@ -18,9 +18,9 @@ body: |
 
     ; CHECK-LABEL: test_ldwrdptr
 
-    ; CHECK:      ld [[SCRATCH:r[0-9]+]], Z+
+    ; CHECK:      ld [[SCRATCH:r[0-9]+]], Z
     ; CHECK-NEXT: push [[SCRATCH]]
-    ; CHECK-NEXT: ld [[SCRATCH]], Z
+    ; CHECK-NEXT: ldd [[SCRATCH]], Z+1
     ; CHECK-NEXT: mov r31, [[SCRATCH]]
     ; CHECK-NEXT: pop r30
 
index 5398a8b1a982ac37ed9b2219be59a492f624b89a..5bd4bf2d431c8b517a4e95c755132575368d7ed1 100644 (file)
@@ -17,8 +17,8 @@ body: |
 
     ; CHECK-LABEL: test_ldwrdptr
 
-    ; CHECK:      $r0, $r31r30 = LDRdPtrPi $r31r30
-    ; CHECK-NEXT:          $r1 = LDRdPtr $r31r30
+    ; CHECK:      $r0, $r31r30 = LDRdPtr
+    ; CHECK-NEXT:          $r1 = LDDRdPtrQ $r31r30, 1
 
     $r1r0 = LDWRdPtr $r31r30
 ...