[AVR] Fix incorrect expansion of the pseudo 'ELPMBRdZ' instruction

[benshi001]

Summary

The ELPM instruction has three forms:

---

form	feature
ELPM	hasELPM
ELPM Rd, Z	hasELPMX
ELPM Rd, Z+	hasELPMX

---

The second form is always used in the expansion of the pseudo
instruction ELPMBRdZ. But for devices without FeatureELPMX and
with only FeatureELPM, only the first form can be used.

[benshi001]

llvm/test/CodeGen/AVR/elpm.ll

296

The only difference between CHECK and NOX is that elpm rd, z is replaced by elpm + mov rd, r0.

[benshi001]

llvm/lib/Target/AVR/AVRInstrInfo.td

1747

ELPMBRdZ does not clobber the Z register.

llvm/test/CodeGen/AVR/elpm.ll

246

The only difference between CHECK and NOX is that elpm rd, z is replaced by elpm + mov rd, r0.

[benshi001]

ping ...

[jacquesguan]

LGTM.

llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp

bool AVRExpandPseudo::expand<AVR::ELPMBRdZ>(Block &MBB, BlockIt MBBI) {
   Register BankReg = MI.getOperand(2).getReg();
   bool SrcIsKill = MI.getOperand(1).isKill();
   const AVRSubtarget &STI = MBB.getParent()->getSubtarget<AVRSubtarget>();
 
   // Set the I/O register RAMPZ for ELPM (out RAMPZ, rtmp).
   buildMI(MBB, MBBI, AVR::OUTARr).addImm(STI.getIORegRAMPZ()).addReg(BankReg);
 
   // Load byte.
+  if (STI.hasELPMX()) {
     auto MILB = buildMI(MBB, MBBI, AVR::ELPMRdZ)
                     .addReg(DstReg, RegState::Define)
                     .addReg(SrcReg, getKillRegState(SrcIsKill));
-
     MILB.setMemRefs(MI.memoperands());
+  } else {
+    // For the basic 'ELPM' instruction, its operand[0] is the implicit
+    // 'Z' register, and its operand[1] is the implicit 'R0' register.
+    auto MILB = buildMI(MBB, MBBI, AVR::ELPM);
+    buildMI(MBB, MBBI, AVR::MOVRdRr)
+        .addReg(DstReg, RegState::Define)
+        .addReg(AVR::R0, RegState::Kill);
+    MILB.setMemRefs(MI.memoperands());
+  }
 
   MI.eraseFromParent();
   return true;
 }
 
 template <>
 bool AVRExpandPseudo::expand<AVR::LPMWRdZPi>(Block &MBB, BlockIt MBBI) {
   llvm_unreachable("16-bit LPMPi is unimplemented");

llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp

template <> bool AVRDAGToDAGISel::select<ISD::LOAD>(SDNode *N) {
   }
 
   if (!Subtarget->hasLPM())
     report_fatal_error("cannot load from program memory on this mcu");
 
   int ProgMemBank = AVR::getProgramMemoryBank(LD);
   if (ProgMemBank < 0 || ProgMemBank > 5)
     report_fatal_error("unexpected program memory bank");
+  if (ProgMemBank > 0 && !Subtarget->hasELPM())
+    report_fatal_error("unexpected program memory bank");
 
   // This is a flash memory load, move the pointer into R31R30 and emit
   // the lpm instruction.
   MVT VT = LD->getMemoryVT().getSimpleVT();
   SDValue Chain = LD->getChain();
   SDValue Ptr = LD->getBasePtr();
   SDNode *ResNode;
   SDLoc DL(N);

llvm/lib/Target/AVR/AVRInstrInfo.td

def ELPMRdZ : FLPMX<1, 0, (outs GPR8:$rd), (ins ZREG:$z),
                 Requires<[HasELPMX]>;
 
   let Defs = [R31R30] in {
     def ELPMRdZPi : FLPMX<1, 1, (outs GPR8:$rd), (ins ZREG:$z),
                           "elpm\t$rd, $z+", []>,
                     Requires<[HasELPMX]>;
   }
 
-  // These pseudos are combination of the OUT and ELPM instructions.
-  let Defs = [R31R30], hasSideEffects = 1 in {
-    def ELPMBRdZ : Pseudo<(outs GPR8:$dst), (ins ZREG:$z, LD8:$p),
-                          "elpmb\t$dst, $z, $p", []>,
-                   Requires<[HasELPMX]>;
+  // This pseudo is combination of the OUT and ELPM instructions.
+  let Defs = [R0] in
+                 Requires<[HasELPM]>;
 
+  // These pseudos are combination of the OUT and ELPM instructions.
+  let Defs = [R31R30], hasSideEffects = 1 in {
     let Constraints = "@earlyclobber $dst" in
     def ELPMWRdZ : Pseudo<(outs DREGS:$dst), (ins ZREG:$z, LD8:$p),
                           "elpmw\t$dst, $z, $p", []>,
                    Requires<[HasELPMX]>;
 
     def ELPMBRdZPi : Pseudo<(outs GPR8:$dst), (ins ZREG:$z, LD8:$p),
                             "elpmb\t$dst, $z+, $p", []>,
                      Requires<[HasELPMX]>;

llvm/test/CodeGen/AVR/elpm.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=avr --mcpu=atmega2560 -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=avr -mattr=+movw -mattr=+elpm -mattr=+elpmx -mattr=+lpm -mattr=+lpmx -verify-machineinstrs \
+; RUN:     | FileCheck %s
+; RUN: llc < %s -mtriple=avr -mattr=+movw -mattr=+elpm -mattr=-elpmx -mattr=+lpm -mattr=-lpmx -verify-machineinstrs \
+; RUN:     | FileCheck --check-prefix=NOX %s
 
 @arr0 = addrspace(1) constant [4 x i16] [i16 123, i16 24, i16 56, i16 37], align 1
 @arr1 = addrspace(2) constant [4 x i16] [i16 123, i16 34, i16 46, i16 27], align 1
 @arr2 = addrspace(3) constant [4 x i16] [i16 123, i16 23, i16 45, i16 17], align 1
 
 define i16 @foo0(i16 %a, i16 %b) {
 ; CHECK-LABEL: foo0:
 ; CHECK:       ; %bb.0: ; %entry
entry:
   %arrayidx = getelementptr inbounds [4 x i16], [4 x i16] addrspace(3)* @arr2, i16 0, i16 %a
   %0 = load i16, i16 addrspace(3)* %arrayidx, align 1
   %arrayidx1 = getelementptr inbounds [4 x i16], [4 x i16] addrspace(2)* @arr1, i16 0, i16 %b
   %1 = load i16, i16 addrspace(2)* %arrayidx1, align 1
   %sub = sub nsw i16 %0, %1
   ret i16 %sub
 }
 
-@arrb1 = addrspace(1) constant [4 x i8] c"{\188%", align 1
-@arrb3 = addrspace(3) constant [4 x i8] c"{\22.\1B", align 1
-@arrb5 = addrspace(5) constant [4 x i8] c"{\17-\11", align 1
+@arrb1 = addrspace(1) constant [4 x i8] c"abcd", align 1
+@arrb3 = addrspace(3) constant [4 x i8] c"1234", align 1
+@arrb5 = addrspace(5) constant [4 x i8] c"HJLQ", align 1
 
 define signext i8 @foob0(i16 %a, i16 %b) {
 ; CHECK-LABEL: foob0:
 ; CHECK:       ; %bb.0: ; %entry
 ; CHECK-NEXT:    subi r22, lo8(-(arrb1))
 ; CHECK-NEXT:    sbci r23, hi8(-(arrb1))
 ; CHECK-NEXT:    movw r30, r22
 ; CHECK-NEXT:    lpm r18, Z

 ; CHECK-NEXT:    ldi r24, 2
 ; CHECK-NEXT:    out 59, r24
 ; CHECK-NEXT:    elpm r24, Z
 ; CHECK-NEXT:    sub r24, r18
 ; CHECK-NEXT:    mov r25, r24
 ; CHECK-NEXT:    lsl r25
 ; CHECK-NEXT:    sbc r25, r25
 ; CHECK-NEXT:    ret
+;
+; NOX-LABEL: foob3:
+; NOX:       ; %bb.0: ; %entry
+; NOX-NEXT:    subi r22, lo8(-(arrb5))
+; NOX-NEXT:    sbci r23, hi8(-(arrb5))
+; NOX-NEXT:    movw r30, r22
+; NOX-NEXT:    ldi r18, 4
+; NOX-NEXT:    out 59, r18
+; NOX-NEXT:    elpm
+; NOX-NEXT:    mov r18, r0
+; NOX-NEXT:    subi r24, lo8(-(arrb3))
+; NOX-NEXT:    sbci r25, hi8(-(arrb3))
+; NOX-NEXT:    movw r30, r24
+; NOX-NEXT:    ldi r24, 2
+; NOX-NEXT:    out 59, r24
+; NOX-NEXT:    elpm
+; NOX-NEXT:    mov r24, r0
+; NOX-NEXT:    sub r24, r18
+; NOX-NEXT:    mov r25, r24
+; NOX-NEXT:    lsl r25
+; NOX-NEXT:    sbc r25, r25
+; NOX-NEXT:    ret
 entry:
   %arrayidx = getelementptr inbounds [4 x i8], [4 x i8] addrspace(3)* @arrb3, i16 0, i16 %a
   %0 = load i8, i8 addrspace(3)* %arrayidx, align 1
   %arrayidx1 = getelementptr inbounds [4 x i8], [4 x i8] addrspace(5)* @arrb5, i16 0, i16 %b
   %1 = load i8, i8 addrspace(5)* %arrayidx1, align 1
   %sub = sub i8 %0, %1
   ret i8 %sub
 }

 ; CHECK-NEXT:    movw r30, r24
 ; CHECK-NEXT:    out 59, r18
 ; CHECK-NEXT:    elpm r24, Z
 ; CHECK-NEXT:    sub r24, r19
 ; CHECK-NEXT:    mov r25, r24
 ; CHECK-NEXT:    lsl r25
 ; CHECK-NEXT:    sbc r25, r25
 ; CHECK-NEXT:    ret
+;
+; NOX-LABEL: foob4:
+; NOX:       ; %bb.0: ; %entry
+; NOX-NEXT:    subi r22, lo8(-(arrb3))
+; NOX-NEXT:    sbci r23, hi8(-(arrb3))
+; NOX-NEXT:    movw r30, r22
+; NOX-NEXT:    ldi r18, 2
+; NOX-NEXT:    out 59, r18
+; NOX-NEXT:    elpm
+; NOX-NEXT:    mov r19, r0
+; NOX-NEXT:    subi r24, lo8(-(arrb3))
+; NOX-NEXT:    sbci r25, hi8(-(arrb3))
+; NOX-NEXT:    movw r30, r24
+; NOX-NEXT:    out 59, r18
+; NOX-NEXT:    elpm
+; NOX-NEXT:    mov r24, r0
+; NOX-NEXT:    sub r24, r19
+; NOX-NEXT:    mov r25, r24
+; NOX-NEXT:    lsl r25
+; NOX-NEXT:    sbc r25, r25
+; NOX-NEXT:    ret
 entry:
   %arrayidx = getelementptr inbounds [4 x i8], [4 x i8] addrspace(3)* @arrb3, i16 0, i16 %a
   %0 = load i8, i8 addrspace(3)* %arrayidx, align 1
   %arrayidx1 = getelementptr inbounds [4 x i8], [4 x i8] addrspace(3)* @arrb3, i16 0, i16 %b
   %1 = load i8, i8 addrspace(3)* %arrayidx1, align 1
   %sub = sub i8 %0, %1
   ret i8 %sub
 }

llvm/test/CodeGen/AVR/pseudo/ELPMBRdZ.mir

+# RUN: llc -mtriple=avr -mattr=+elpm -mattr=+elpmx -start-before=greedy %s -o - \
+# RUN:     | FileCheck %s
+# RUN: llc -mtriple=avr -mattr=+elpm -mattr=-elpmx -start-before=greedy %s -o - \
+# RUN:     | FileCheck --check-prefix=NOX %s
+
+# This test checks the expansion of the 16-bit ELPM pseudo instruction and that
+# the register allocator won't use R31R30 as an output register (which would
+# lead to undefined behavior).
+
+--- |
+  target triple = "avr--"
+  define void @test_elpmbrdz() {
+  entry:
+    ret void
+  }
+...
+
+---
+name:            test_elpmbrdz
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $r31r30
+
+    ; CHECK-LABEL: test_elpmbrdz
+    ; CHECK:       ; %bb.0:
+    ; CHECK-NEXT:    ldi r24, 1
+    ; CHECK-NEXT:    out
+    ; CHECK-NEXT:    elpm r31, Z
+    ; CHECK-NEXT:    ret
+
+    ; NOX-LABEL:   test_elpmbrdz
+    ; NOX:         ; %bb.0:
+    ; NOX-NEXT:      ldi r24, 1
+    ; NOX-NEXT:      out
+    ; NOX-NEXT:      elpm
+    ; NOX-NEXT:      mov r31, r0
+    ; NOX-NEXT:      ret
+
+    %1:zreg = COPY killed $r31r30
+    %2:ld8 = LDIRdK 1
+    %3:gpr8 = ELPMBRdZ %1, %2, implicit-def dead $r0
+    $r31 = COPY %3
+    RET implicit $r31
+...