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

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 ELPMX but only
with ELPM, only the first form can be emitted.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D141221

diff --git a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
index 2c97dea..06dc2b7 100644 (file)
--- a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
@@ -871,11 +871,20 @@ bool AVRExpandPseudo::expand<AVR::ELPMBRdZ>(Block &MBB, BlockIt MBBI) {
   buildMI(MBB, MBBI, AVR::OUTARr).addImm(STI.getIORegRAMPZ()).addReg(BankReg);
 
   // Load byte.
-  auto MILB = buildMI(MBB, MBBI, AVR::ELPMRdZ)
-                  .addReg(DstReg, RegState::Define)
-                  .addReg(SrcReg, getKillRegState(SrcIsKill));
-
-  MILB.setMemRefs(MI.memoperands());
+  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;

diff --git a/llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp b/llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
index 5511d53..03015a4 100644 (file)
--- a/llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
+++ b/llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
@@ -366,6 +366,8 @@ template <> bool AVRDAGToDAGISel::select<ISD::LOAD>(SDNode *N) {
   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.

diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.td b/llvm/lib/Target/AVR/AVRInstrInfo.td
index 05ee94b..c272711 100644 (file)
--- a/llvm/lib/Target/AVR/AVRInstrInfo.td
+++ b/llvm/lib/Target/AVR/AVRInstrInfo.td
@@ -1742,12 +1742,14 @@ let mayLoad = 1, hasSideEffects = 0 in {
                     Requires<[HasELPMX]>;
   }
 
+  // This pseudo is combination of the OUT and ELPM instructions.
+  let Defs = [R0] in
+  def ELPMBRdZ : Pseudo<(outs GPR8:$dst), (ins ZREG:$z, LD8:$p),
+                        "elpmb\t$dst, $z, $p", []>,
+                 Requires<[HasELPM]>;
+
   // 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]>;
-
     let Constraints = "@earlyclobber $dst" in
     def ELPMWRdZ : Pseudo<(outs DREGS:$dst), (ins ZREG:$z, LD8:$p),
                           "elpmw\t$dst, $z, $p", []>,

diff --git a/llvm/test/CodeGen/AVR/elpm.ll b/llvm/test/CodeGen/AVR/elpm.ll
index a322ab7..ba28bc8 100644 (file)
--- a/llvm/test/CodeGen/AVR/elpm.ll
+++ b/llvm/test/CodeGen/AVR/elpm.ll
@@ -1,5 +1,8 @@
 ; 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
@@ -129,9 +132,9 @@ entry:
   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:
@@ -232,6 +235,28 @@ define signext i8 @foob3(i16 %a, i16 %b) {
 ; 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
@@ -260,6 +285,27 @@ define signext i8 @foob4(i16 %a, i16 %b) {
 ; 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

diff --git a/llvm/test/CodeGen/AVR/pseudo/ELPMBRdZ.mir b/llvm/test/CodeGen/AVR/pseudo/ELPMBRdZ.mir
new file mode 100644 (file)
index 0000000..29dbd79
--- /dev/null
+++ b/llvm/test/CodeGen/AVR/pseudo/ELPMBRdZ.mir
@@ -0,0 +1,45 @@
+# 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
+...