[llvm] 4fa9dc9 - [AVR] Fix incorrect expansion of the pseudo 'ELPMBRdZ' instruction

Ben Shi via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 20 20:37:51 PDT 2023


Author: Ben Shi
Date: 2023-03-21T11:33:56+08:00
New Revision: 4fa9dc948226e374372537250d046924d348307e

URL: https://github.com/llvm/llvm-project/commit/4fa9dc948226e374372537250d046924d348307e
DIFF: https://github.com/llvm/llvm-project/commit/4fa9dc948226e374372537250d046924d348307e.diff

LOG: [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

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

Modified: 
    llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
    llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp
    llvm/lib/Target/AVR/AVRInstrInfo.td
    llvm/test/CodeGen/AVR/elpm.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
index 2c97dea0bce03..06dc2b7c5b27b 100644
--- 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 5511d53dfa312..03015a457a0d1 100644
--- 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 05ee94be79263..c272711bb8663 100644
--- 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 a322ab773014a..ba28bc814591d 100644
--- 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
 }
 
- at arrb1 = addrspace(1) constant [4 x i8] c"{\188%", align 1
- at arrb3 = addrspace(3) constant [4 x i8] c"{\22.\1B", align 1
- at arrb5 = addrspace(5) constant [4 x i8] c"{\17-\11", align 1
+ at arrb1 = addrspace(1) constant [4 x i8] c"abcd", align 1
+ at arrb3 = addrspace(3) constant [4 x i8] c"1234", align 1
+ at 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
index 0000000000000..29dbd79c652a2
--- /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
+...


        


More information about the llvm-commits mailing list