[llvm] 15f495c - [AVR] Fix def state of operands

[Ayke van Laethem via llvm-commits]

Author: Ayke van Laethem
Date: 2021-03-03T15:36:05+01:00
New Revision: 15f495c0bcb2fcab282582d9a50e234e4103cd1a

URL: https://github.com/llvm/llvm-project/commit/15f495c0bcb2fcab282582d9a50e234e4103cd1a
DIFF: https://github.com/llvm/llvm-project/commit/15f495c0bcb2fcab282582d9a50e234e4103cd1a.diff

LOG: [AVR] Fix def state of operands

Some instructions (especially mov+pop instructions) were setting the
wrong operands. For example, the pop instruction had the register set as
a source operand while it is a destination operand (the value is loaded
into the register).

I have found these issues using the machine verifier and using manual
code inspection.

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

Added: 
    

Modified: 
    llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
    llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
    llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
index f266bdbb4cfa..8fb020ce9c62 100644
--- a/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp
@@ -650,10 +650,10 @@ bool AVRExpandPseudo::expand<AVR::LDWRdPtr>(Block &MBB, BlockIt MBBI) {
 
   if (TmpReg) {
     // Move the high byte into the final destination.
-    buildMI(MBB, MBBI, AVR::MOVRdRr).addReg(DstHiReg).addReg(TmpReg);
+    buildMI(MBB, MBBI, AVR::MOVRdRr, DstHiReg).addReg(TmpReg);
 
     // Move the low byte from the scratch space into the final destination.
-    buildMI(MBB, MBBI, AVR::POPRd).addReg(DstLoReg);
+    buildMI(MBB, MBBI, AVR::POPRd, DstLoReg);
   }
 
   MIBLO.setMemRefs(MI.memoperands());
@@ -767,10 +767,10 @@ bool AVRExpandPseudo::expand<AVR::LDDWRdPtrQ>(Block &MBB, BlockIt MBBI) {
 
   if (TmpReg) {
     // Move the high byte into the final destination.
-    buildMI(MBB, MBBI, AVR::MOVRdRr).addReg(DstHiReg).addReg(TmpReg);
+    buildMI(MBB, MBBI, AVR::MOVRdRr, DstHiReg).addReg(TmpReg);
 
     // Move the low byte from the scratch space into the final destination.
-    buildMI(MBB, MBBI, AVR::POPRd).addReg(DstLoReg);
+    buildMI(MBB, MBBI, AVR::POPRd, DstLoReg);
   }
 
   MIBLO.setMemRefs(MI.memoperands());
@@ -815,10 +815,10 @@ bool AVRExpandPseudo::expand<AVR::LPMWRdZ>(Block &MBB, BlockIt MBBI) {
 
   if (TmpReg) {
     // Move the high byte into the final destination.
-    buildMI(MBB, MBBI, AVR::MOVRdRr).addReg(DstHiReg).addReg(TmpReg);
+    buildMI(MBB, MBBI, AVR::MOVRdRr, DstHiReg).addReg(TmpReg);
 
     // Move the low byte from the scratch space into the final destination.
-    buildMI(MBB, MBBI, AVR::POPRd).addReg(DstLoReg);
+    buildMI(MBB, MBBI, AVR::POPRd, DstLoReg);
   }
 
   MIBLO.setMemRefs(MI.memoperands());
@@ -1750,8 +1750,9 @@ bool AVRExpandPseudo::expand<AVR::ASRW8Rd>(Block &MBB, BlockIt MBBI) {
       .addReg(DstHiReg, getKillRegState(DstIsKill));
 
   // Move the sign bit to the C flag.
-  buildMI(MBB, MBBI, AVR::ADDRdRr).addReg(DstHiReg)
-      .addReg(DstHiReg, RegState::Define | getDeadRegState(DstIsDead))
+  buildMI(MBB, MBBI, AVR::ADDRdRr)
+      .addReg(DstHiReg, RegState::Define, getDeadRegState(DstIsDead))
+      .addReg(DstHiReg, getKillRegState(DstIsKill) | getDeadRegState(DstIsDead))
       .addReg(DstHiReg, getKillRegState(DstIsKill));
 
   // Set upper byte to 0 or -1.

diff  --git a/llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp b/llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
index 6be901743e82..7d2d19de7578 100644
--- a/llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
+++ b/llvm/lib/Target/AVR/AVRRelaxMemOperations.cpp
@@ -113,7 +113,7 @@ bool AVRRelaxMem::relax<AVR::STDWPtrQRr>(Block &MBB, BlockIt MBBI) {
 
     // Pop the original state of the pointer register.
     buildMI(MBB, MBBI, AVR::POPWRd)
-      .addReg(Ptr.getReg(), getKillRegState(Ptr.isKill()));
+      .addDef(Ptr.getReg(), getKillRegState(Ptr.isKill()));
 
     MI.removeFromParent();
   }

diff  --git a/llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir b/llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
index b97c6d3be252..56f9e428f365 100644
--- a/llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
+++ b/llvm/test/CodeGen/AVR/relax-mem/STDWPtrQRr.mir
@@ -26,6 +26,6 @@ body: |
     ; CHECK-NEXT: PUSHWRr $r29r28, implicit-def $sp, implicit $sp
     ; CHECK-NEXT: $r29r28 = SBCIWRdK $r29r28, -64, implicit-def $sreg, implicit $sreg
     ; CHECK-NEXT: STWPtrRr $r29r28, $r1r0
-    ; CHECK-NEXT: POPWRd $r29r28, implicit-def $sp, implicit $sp
+    ; CHECK-NEXT: $r29r28 = POPWRd implicit-def $sp, implicit $sp
     STDWPtrQRr $r29r28, 64, $r1r0
 ...