[llvm] r351673 - [AVR] Fix codegen bug in 16-bit loads

Dylan McKay via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 19 19:41:08 PST 2019


Author: dylanmckay
Date: Sat Jan 19 19:41:08 2019
New Revision: 351673

URL: http://llvm.org/viewvc/llvm-project?rev=351673&view=rev
Log:
[AVR] Fix codegen bug in 16-bit loads

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.

Added:
    llvm/trunk/test/CodeGen/AVR/PR37143.ll
Modified:
    llvm/trunk/lib/Target/AVR/AVRExpandPseudoInsts.cpp
    llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp
    llvm/trunk/test/CodeGen/AVR/atomics/load16.ll
    llvm/trunk/test/CodeGen/AVR/load.ll
    llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr-same-src-dst.mir
    llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr.mir

Modified: llvm/trunk/lib/Target/AVR/AVRExpandPseudoInsts.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AVR/AVRExpandPseudoInsts.cpp?rev=351673&r1=351672&r2=351673&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AVR/AVRExpandPseudoInsts.cpp (original)
+++ llvm/trunk/lib/Target/AVR/AVRExpandPseudoInsts.cpp Sat Jan 19 19:41:08 2019
@@ -582,8 +582,8 @@ bool AVRExpandPseudo::expand<AVR::LDWRdP
   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::LDWRdP
   // 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::LDWRdP
   // 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.

Modified: llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp?rev=351673&r1=351672&r2=351673&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp Sat Jan 19 19:41:08 2019
@@ -1278,7 +1278,6 @@ SDValue AVRTargetLowering::LowerCall(Tar
   }
 
   // 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::in
   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(MachineBasic
 // 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

Added: llvm/trunk/test/CodeGen/AVR/PR37143.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AVR/PR37143.ll?rev=351673&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AVR/PR37143.ll (added)
+++ llvm/trunk/test/CodeGen/AVR/PR37143.ll Sat Jan 19 19:41:08 2019
@@ -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
+}

Modified: llvm/trunk/test/CodeGen/AVR/atomics/load16.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AVR/atomics/load16.ll?rev=351673&r1=351672&r2=351673&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AVR/atomics/load16.ll (original)
+++ llvm/trunk/test/CodeGen/AVR/atomics/load16.ll Sat Jan 19 19:41:08 2019
@@ -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*
 ; 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

Modified: llvm/trunk/test/CodeGen/AVR/load.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AVR/load.ll?rev=351673&r1=351672&r2=351673&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AVR/load.ll (original)
+++ llvm/trunk/test/CodeGen/AVR/load.ll Sat Jan 19 19:41:08 2019
@@ -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

Modified: llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr-same-src-dst.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr-same-src-dst.mir?rev=351673&r1=351672&r2=351673&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr-same-src-dst.mir (original)
+++ llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr-same-src-dst.mir Sat Jan 19 19:41:08 2019
@@ -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
 

Modified: llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr.mir?rev=351673&r1=351672&r2=351673&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr.mir (original)
+++ llvm/trunk/test/CodeGen/AVR/pseudo/LDWRdPtr.mir Sat Jan 19 19:41:08 2019
@@ -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
 ...




More information about the llvm-commits mailing list