[AVR] Do not emit 'LPM Rd, Z' on devices without 'FeatureLPMX'

[benshi001]

Summary

The LPM instruction has three forms:

---

form	feature
LPM	hasLPM
LPM Rd, Z	hasLPMX
LPM Rd, Z+	hasLPMX

---

The second form is always selected in ISelDAGToDAG, even on devices
without FeatureLPMX. This patch emits "LPM + MOV" on devices with
only FeatureLPM.

[benshi001]

llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp

869

HasX means if there is extended form of LPM/ELPM.
IsExt means it is E(xtended)LPM, not (ordinray) LPM.

llvm/test/CodeGen/AVR/elpm.ll

161

The only difference between CHECK and NOX is that lpm Rd, Z is replaced by lpm + mov Rd, R0.

[jacquesguan]

LGTM.

llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp

private:
   bool expandASRW8Rd(Block &MBB, BlockIt MBBI);
   bool expandLSLW12Rd(Block &MBB, BlockIt MBBI);
   bool expandLSRW12Rd(Block &MBB, BlockIt MBBI);
   bool expandASRW14Rd(Block &MBB, BlockIt MBBI);
   bool expandASRW15Rd(Block &MBB, BlockIt MBBI);
 
   // Common implementation of LPMWRdZ and ELPMWRdZ.
   bool expandLPMWELPMW(Block &MBB, BlockIt MBBI, bool IsExt);
+  // Common implementation of LPMBRdZ and ELPMBRdZ.
+  bool expandLPMBELPMB(Block &MBB, BlockIt MBBI, bool IsExt);
 };
 
 char AVRExpandPseudo::ID = 0;
 
 bool AVRExpandPseudo::expandMBB(MachineBasicBlock &MBB) {
   bool Modified = false;
 
   BlockIt MBBI = MBB.begin(), E = MBB.end();
bool AVRExpandPseudo::expand<AVR::LPMWRdZ>(Block &MBB, BlockIt MBBI) {
   return expandLPMWELPMW(MBB, MBBI, false);
 }
 
 template <>
 bool AVRExpandPseudo::expand<AVR::ELPMWRdZ>(Block &MBB, BlockIt MBBI) {
   return expandLPMWELPMW(MBB, MBBI, true);
 }
 
-template <>
-bool AVRExpandPseudo::expand<AVR::ELPMBRdZ>(Block &MBB, BlockIt MBBI) {
+bool AVRExpandPseudo::expandLPMBELPMB(Block &MBB, BlockIt MBBI, bool IsExt) {
   MachineInstr &MI = *MBBI;
   Register DstReg = MI.getOperand(0).getReg();
   Register SrcReg = MI.getOperand(1).getReg();
-  Register BankReg = MI.getOperand(2).getReg();
   bool SrcIsKill = MI.getOperand(1).isKill();
   const AVRSubtarget &STI = MBB.getParent()->getSubtarget<AVRSubtarget>();
+  bool HasX = IsExt ? STI.hasELPMX() : STI.hasLPMX();
 
   // Set the I/O register RAMPZ for ELPM (out RAMPZ, rtmp).
+  if (IsExt) {
+    Register BankReg = MI.getOperand(2).getReg();
     buildMI(MBB, MBBI, AVR::OUTARr).addImm(STI.getIORegRAMPZ()).addReg(BankReg);
+  }
 
   // Load byte.
-  if (STI.hasELPMX()) {
-    auto MILB = buildMI(MBB, MBBI, AVR::ELPMRdZ)
+  if (HasX) {
+    unsigned Opc = IsExt ? AVR::ELPMRdZ : AVR::LPMRdZ;
+    auto MILB = buildMI(MBB, MBBI, Opc)
                     .addReg(DstReg, RegState::Define)
                     .addReg(SrcReg, getKillRegState(SrcIsKill));
     MILB.setMemRefs(MI.memoperands());
   } else {
-    // For the basic 'ELPM' instruction, its operand[0] is the implicit
+    // For the basic ELPM/LPM 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);
+    unsigned Opc = IsExt ? AVR::ELPM : AVR::LPM;
+    auto MILB = buildMI(MBB, MBBI, Opc);
     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::ELPMBRdZ>(Block &MBB, BlockIt MBBI) {
+  return expandLPMBELPMB(MBB, MBBI, true);
+}
+
+template <>
+bool AVRExpandPseudo::expand<AVR::LPMBRdZ>(Block &MBB, BlockIt MBBI) {
+  return expandLPMBELPMB(MBB, MBBI, false);
+}
+
+template <>
 bool AVRExpandPseudo::expand<AVR::LPMWRdZPi>(Block &MBB, BlockIt MBBI) {
   llvm_unreachable("16-bit LPMPi is unimplemented");
 }
 
 template <>
 bool AVRExpandPseudo::expand<AVR::ELPMBRdZPi>(Block &MBB, BlockIt MBBI) {
   llvm_unreachable("8-bit ELPMPi is unimplemented");
 }
switch (Opcode) {
     EXPAND(AVR::CPCWRdRr);
     EXPAND(AVR::LDIWRdK);
     EXPAND(AVR::LDSWRdK);
     EXPAND(AVR::LDWRdPtr);
     EXPAND(AVR::LDWRdPtrPi);
     EXPAND(AVR::LDWRdPtrPd);
   case AVR::LDDWRdYQ: //: FIXME: remove this once PR13375 gets fixed
     EXPAND(AVR::LDDWRdPtrQ);
+    EXPAND(AVR::LPMBRdZ);
     EXPAND(AVR::LPMWRdZ);
     EXPAND(AVR::LPMWRdZPi);
     EXPAND(AVR::ELPMBRdZ);
     EXPAND(AVR::ELPMWRdZ);
     EXPAND(AVR::ELPMBRdZPi);
     EXPAND(AVR::ELPMWRdZPi);
     EXPAND(AVR::AtomicLoad8);
     EXPAND(AVR::AtomicLoad16);

llvm/lib/Target/AVR/AVRISelDAGToDAG.cpp

if (ProgMemBank == 0) {
       ResNode = CurDAG->getMachineNode(LPMOpc, DL, VT, MVT::i16, MVT::Other,
                                        Ptr, SDValue(NP, 0));
     }
   } else {
     // Selecting an indexed load is not legal, fallback to a normal load.
     switch (VT.SimpleTy) {
     case MVT::i8:
       if (ProgMemBank == 0) {
+        unsigned Opc = Subtarget->hasLPMX() ? AVR::LPMRdZ : AVR::LPMBRdZ;
         ResNode =
-            CurDAG->getMachineNode(AVR::LPMRdZ, DL, MVT::i8, MVT::Other, Ptr);
+            CurDAG->getMachineNode(Opc, DL, MVT::i8, MVT::Other, Ptr);
       } else {
         // Do not combine the LDI instruction into the ELPM pseudo instruction,
         // since it may be reused by other ELPM pseudo instructions.
         SDValue NC = CurDAG->getTargetConstant(ProgMemBank, DL, MVT::i8);
         auto *NP = CurDAG->getMachineNode(AVR::LDIRdK, DL, MVT::i8, NC);
         ResNode = CurDAG->getMachineNode(AVR::ELPMBRdZ, DL, MVT::i8, MVT::Other,
                                          Ptr, SDValue(NP, 0));
       }

llvm/lib/Target/AVR/AVRInstrInfo.td

 // Load program memory operations.
 let canFoldAsLoad = 1, isReMaterializable = 1, mayLoad = 1,
     hasSideEffects = 0 in {
   let Defs = [R0],
       Uses = [R31R30] in def LPM
       : F16<0b1001010111001000, (outs), (ins), "lpm", []>,
       Requires<[HasLPM]>;
 
+  // This pseudo is combination of LPM and MOV instructions.
+  let Defs = [R0] in
+  def LPMBRdZ : Pseudo<(outs GPR8:$dst), (ins ZREG:$z), "lpmb\t$dst, $z", []>,
+                Requires<[HasLPM]>;
+
   def LPMRdZ : FLPMX<0, 0,
                      (outs GPR8
                       : $rd),
                      (ins ZREG
                       : $z),
                      "lpm\t$rd, $z", []>,
                Requires<[HasLPMX]>;
 

llvm/test/CodeGen/AVR/elpm.ll

 ; CHECK-NEXT:    sbci r25, hi8(-(arrb1))
 ; CHECK-NEXT:    movw r30, r24
 ; CHECK-NEXT:    lpm 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: foob0:
+; NOX:       ; %bb.0: ; %entry
+; NOX-NEXT:    subi r22, lo8(-(arrb1))
+; NOX-NEXT:    sbci r23, hi8(-(arrb1))
+; NOX-NEXT:    movw r30, r22
+; NOX-NEXT:    lpm
+; NOX-NEXT:    mov r18, r0
+; NOX-NEXT:    subi r24, lo8(-(arrb1))
+; NOX-NEXT:    sbci r25, hi8(-(arrb1))
+; NOX-NEXT:    movw r30, r24
+; NOX-NEXT:    lpm
+; 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(1)* @arrb1, i16 0, i16 %a
   %0 = load i8, i8 addrspace(1)* %arrayidx, align 1
   %arrayidx1 = getelementptr inbounds [4 x i8], [4 x i8] addrspace(1)* @arrb1, i16 0, i16 %b
   %1 = load i8, i8 addrspace(1)* %arrayidx1, align 1
   %sub = sub i8 %0, %1
   ret i8 %sub
 }

 ; CHECK-NEXT:    sbci r25, hi8(-(arrb1))
 ; CHECK-NEXT:    movw r30, r24
 ; CHECK-NEXT:    lpm 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: foob1:
+; 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 r18, r0
+; NOX-NEXT:    subi r24, lo8(-(arrb1))
+; NOX-NEXT:    sbci r25, hi8(-(arrb1))
+; NOX-NEXT:    movw r30, r24
+; NOX-NEXT:    lpm
+; 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(1)* @arrb1, i16 0, i16 %a
   %0 = load i8, i8 addrspace(1)* %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
 }

 ; CHECK-NEXT:    sbci r23, hi8(-(arrb1))
 ; CHECK-NEXT:    movw r30, r22
 ; CHECK-NEXT:    lpm r25, Z
 ; CHECK-NEXT:    sub r24, r25
 ; CHECK-NEXT:    mov r25, r24
 ; CHECK-NEXT:    lsl r25
 ; CHECK-NEXT:    sbc r25, r25
 ; CHECK-NEXT:    ret
+;
+; NOX-LABEL: foob2:
+; NOX:       ; %bb.0: ; %entry
+; NOX-NEXT:    subi r24, lo8(-(arrb5))
+; NOX-NEXT:    sbci r25, hi8(-(arrb5))
+; NOX-NEXT:    movw r30, r24
+; NOX-NEXT:    ldi r24, 4
+; NOX-NEXT:    out 59, r24
+; NOX-NEXT:    elpm
+; NOX-NEXT:    mov r24, r0
+; NOX-NEXT:    subi r22, lo8(-(arrb1))
+; NOX-NEXT:    sbci r23, hi8(-(arrb1))
+; NOX-NEXT:    movw r30, r22
+; NOX-NEXT:    lpm
+; NOX-NEXT:    mov r25, r0
+; NOX-NEXT:    sub r24, r25
+; 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(5)* @arrb5, i16 0, i16 %a
   %0 = load i8, i8 addrspace(5)* %arrayidx, align 1
   %arrayidx1 = getelementptr inbounds [4 x i8], [4 x i8] addrspace(1)* @arrb1, i16 0, i16 %b
   %1 = load i8, i8 addrspace(1)* %arrayidx1, align 1
   %sub = sub i8 %0, %1
   ret i8 %sub
 }

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

+# RUN: llc -mtriple=avr -mattr=+lpm -mattr=-lpmx -start-before=greedy %s -o - \
+# RUN:     | FileCheck %s
+
+# This test checks the expansion of the 8-bit LPMBRdZ 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_lpmbrdz() {
+  entry:
+    ret void
+  }
+...
+
+---
+name:            test_lpmbrdz
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $r31r30
+
+    ; CHECK-LABEL: test_lpmbrdz
+    ; CHECK:       ; %bb.0
+    ; CHECK-NEXT:    lpm
+    ; CHECK-NEXT:    mov r30, r0
+
+    %1:zreg = COPY killed $r31r30
+    %2:gpr8 = LPMBRdZ %1, implicit-def dead $r0
+    $r30 = COPY %2
+    RET implicit $r30
+...