[llvm] [RISCV] Use RISCVInstrInfo::movImm to implement most of RISCVPostRAExpandPseudo::expandMovImm (PR #70389)

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 26 15:54:01 PDT 2023


https://github.com/topperc created https://github.com/llvm/llvm-project/pull/70389

There are two commits. The first is a bit of refactoring to increase similarity and fix a bug that was already fixed in movImm. The second does the merging

>From 3f5e74276d9c1dbbfafbf771ccd153634ecb082a Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Thu, 26 Oct 2023 15:06:04 -0700
Subject: [PATCH 1/2] [RISCV] Refactor RISCVPostRAExpandPseudo::expandMovImm
 and RISCVInstrInfo::movImm to prepare for merging.

Fix small bug where RISCVPostRAExpandPseudo::expandMovImm set the
kill flag on X0.
---
 llvm/lib/Target/RISCV/RISCVInstrInfo.cpp      |  9 +++---
 .../RISCV/RISCVPostRAExpandPseudoInsts.cpp    | 28 ++++++++-----------
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 0abf302bb25e310..e02069c266d653d 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -748,6 +748,7 @@ void RISCVInstrInfo::movImm(MachineBasicBlock &MBB,
   assert(!Seq.empty());
 
   for (const RISCVMatInt::Inst &Inst : Seq) {
+    unsigned SrcRegState = getKillRegState(SrcReg != RISCV::X0);
     switch (Inst.getOpndKind()) {
     case RISCVMatInt::Imm:
       BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()), DstReg)
@@ -756,19 +757,19 @@ void RISCVInstrInfo::movImm(MachineBasicBlock &MBB,
       break;
     case RISCVMatInt::RegX0:
       BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()), DstReg)
-          .addReg(SrcReg, getKillRegState(SrcReg != RISCV::X0))
+          .addReg(SrcReg, SrcRegState)
           .addReg(RISCV::X0)
           .setMIFlag(Flag);
       break;
     case RISCVMatInt::RegReg:
       BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()), DstReg)
-          .addReg(SrcReg, getKillRegState(SrcReg != RISCV::X0))
-          .addReg(SrcReg, getKillRegState(SrcReg != RISCV::X0))
+          .addReg(SrcReg, SrcRegState)
+          .addReg(SrcReg, SrcRegState)
           .setMIFlag(Flag);
       break;
     case RISCVMatInt::RegImm:
       BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()), DstReg)
-          .addReg(SrcReg, getKillRegState(SrcReg != RISCV::X0))
+          .addReg(SrcReg, SrcRegState)
           .addImm(Inst.getImm())
           .setMIFlag(Flag);
       break;
diff --git a/llvm/lib/Target/RISCV/RISCVPostRAExpandPseudoInsts.cpp b/llvm/lib/Target/RISCV/RISCVPostRAExpandPseudoInsts.cpp
index 407e7cfd6fef830..2a5b693a5b4f59d 100644
--- a/llvm/lib/Target/RISCV/RISCVPostRAExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/RISCV/RISCVPostRAExpandPseudoInsts.cpp
@@ -101,36 +101,32 @@ bool RISCVPostRAExpandPseudo::expandMovImm(MachineBasicBlock &MBB,
 
   for (RISCVMatInt::Inst &Inst : Seq) {
     bool LastItem = ++Num == Seq.size();
+    unsigned DstRegState = getDeadRegState(DstIsDead && LastItem) |
+                           getRenamableRegState(Renamable);
+    unsigned SrcRegState = getKillRegState(SrcReg != RISCV::X0) |
+                           getRenamableRegState(SrcRenamable);
     switch (Inst.getOpndKind()) {
     case RISCVMatInt::Imm:
       BuildMI(MBB, MBBI, DL, TII->get(Inst.getOpcode()))
-          .addReg(DstReg, RegState::Define |
-                              getDeadRegState(DstIsDead && LastItem) |
-                              getRenamableRegState(Renamable))
+          .addReg(DstReg, RegState::Define | DstRegState)
           .addImm(Inst.getImm());
       break;
     case RISCVMatInt::RegX0:
       BuildMI(MBB, MBBI, DL, TII->get(Inst.getOpcode()))
-          .addReg(DstReg, RegState::Define |
-                              getDeadRegState(DstIsDead && LastItem) |
-                              getRenamableRegState(Renamable))
-          .addReg(SrcReg, RegState::Kill | getRenamableRegState(SrcRenamable))
+          .addReg(DstReg, RegState::Define | DstRegState)
+          .addReg(SrcReg, SrcRegState)
           .addReg(RISCV::X0);
       break;
     case RISCVMatInt::RegReg:
       BuildMI(MBB, MBBI, DL, TII->get(Inst.getOpcode()))
-          .addReg(DstReg, RegState::Define |
-                              getDeadRegState(DstIsDead && LastItem) |
-                              getRenamableRegState(Renamable))
-          .addReg(SrcReg, RegState::Kill | getRenamableRegState(SrcRenamable))
-          .addReg(SrcReg, RegState::Kill | getRenamableRegState(SrcRenamable));
+          .addReg(DstReg, RegState::Define | DstRegState)
+          .addReg(SrcReg, SrcRegState)
+          .addReg(SrcReg, SrcRegState);
       break;
     case RISCVMatInt::RegImm:
       BuildMI(MBB, MBBI, DL, TII->get(Inst.getOpcode()))
-          .addReg(DstReg, RegState::Define |
-                              getDeadRegState(DstIsDead && LastItem) |
-                              getRenamableRegState(Renamable))
-          .addReg(SrcReg, RegState::Kill | getRenamableRegState(SrcRenamable))
+          .addReg(DstReg, RegState::Define | DstRegState)
+          .addReg(SrcReg, SrcRegState)
           .addImm(Inst.getImm());
       break;
     }

>From 6730ce126c24e505fadace86f20ad5729b0def20 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Thu, 26 Oct 2023 15:30:26 -0700
Subject: [PATCH 2/2] [RISCV] Use RISCVInstrInfo::movImm to implement most of
 RISCVPostRAExpandPseudo::expandMovImm

---
 llvm/lib/Target/RISCV/RISCVInstrInfo.cpp      | 25 ++++++++---
 llvm/lib/Target/RISCV/RISCVInstrInfo.h        |  3 +-
 .../RISCV/RISCVPostRAExpandPseudoInsts.cpp    | 43 ++-----------------
 3 files changed, 25 insertions(+), 46 deletions(-)

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index e02069c266d653d..e9c87f7b7b574cc 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -737,7 +737,8 @@ MachineInstr *RISCVInstrInfo::foldMemoryOperandImpl(
 void RISCVInstrInfo::movImm(MachineBasicBlock &MBB,
                             MachineBasicBlock::iterator MBBI,
                             const DebugLoc &DL, Register DstReg, uint64_t Val,
-                            MachineInstr::MIFlag Flag) const {
+                            MachineInstr::MIFlag Flag, bool DstRenamable,
+                            bool DstIsDead) const {
   Register SrcReg = RISCV::X0;
 
   if (!STI.is64Bit() && !isInt<32>(Val))
@@ -747,28 +748,39 @@ void RISCVInstrInfo::movImm(MachineBasicBlock &MBB,
       RISCVMatInt::generateInstSeq(Val, STI.getFeatureBits());
   assert(!Seq.empty());
 
+  bool SrcRenamable = false;
+  unsigned Num = 0;
+
   for (const RISCVMatInt::Inst &Inst : Seq) {
-    unsigned SrcRegState = getKillRegState(SrcReg != RISCV::X0);
+    bool LastItem = ++Num == Seq.size();
+    unsigned DstRegState = getDeadRegState(DstIsDead && LastItem) |
+                           getRenamableRegState(DstRenamable);
+    unsigned SrcRegState = getKillRegState(SrcReg != RISCV::X0) |
+                           getRenamableRegState(SrcRenamable);
     switch (Inst.getOpndKind()) {
     case RISCVMatInt::Imm:
-      BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()), DstReg)
+      BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()))
+          .addReg(DstReg, RegState::Define | DstRegState)
           .addImm(Inst.getImm())
           .setMIFlag(Flag);
       break;
     case RISCVMatInt::RegX0:
-      BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()), DstReg)
+      BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()))
+          .addReg(DstReg, RegState::Define | DstRegState)
           .addReg(SrcReg, SrcRegState)
           .addReg(RISCV::X0)
           .setMIFlag(Flag);
       break;
     case RISCVMatInt::RegReg:
-      BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()), DstReg)
+      BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()))
+          .addReg(DstReg, RegState::Define | DstRegState)
           .addReg(SrcReg, SrcRegState)
           .addReg(SrcReg, SrcRegState)
           .setMIFlag(Flag);
       break;
     case RISCVMatInt::RegImm:
-      BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()), DstReg)
+      BuildMI(MBB, MBBI, DL, get(Inst.getOpcode()))
+          .addReg(DstReg, RegState::Define | DstRegState)
           .addReg(SrcReg, SrcRegState)
           .addImm(Inst.getImm())
           .setMIFlag(Flag);
@@ -777,6 +789,7 @@ void RISCVInstrInfo::movImm(MachineBasicBlock &MBB,
 
     // Only the first instruction has X0 as its source.
     SrcReg = DstReg;
+    SrcRenamable = DstRenamable;
   }
 }
 
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index 5584e5571c9bc35..5c33c5b6a5a1b2a 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -91,7 +91,8 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
   // Materializes the given integer Val into DstReg.
   void movImm(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
               const DebugLoc &DL, Register DstReg, uint64_t Val,
-              MachineInstr::MIFlag Flag = MachineInstr::NoFlags) const;
+              MachineInstr::MIFlag Flag = MachineInstr::NoFlags,
+              bool DstRenamable = false, bool DstIsDead = false) const;
 
   unsigned getInstSizeInBytes(const MachineInstr &MI) const override;
 
diff --git a/llvm/lib/Target/RISCV/RISCVPostRAExpandPseudoInsts.cpp b/llvm/lib/Target/RISCV/RISCVPostRAExpandPseudoInsts.cpp
index 2a5b693a5b4f59d..bc9b66d6ca6b114 100644
--- a/llvm/lib/Target/RISCV/RISCVPostRAExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/RISCV/RISCVPostRAExpandPseudoInsts.cpp
@@ -92,48 +92,13 @@ bool RISCVPostRAExpandPseudo::expandMovImm(MachineBasicBlock &MBB,
       Val, MBB.getParent()->getSubtarget().getFeatureBits());
   assert(!Seq.empty());
 
-  Register SrcReg = RISCV::X0;
   Register DstReg = MBBI->getOperand(0).getReg();
   bool DstIsDead = MBBI->getOperand(0).isDead();
   bool Renamable = MBBI->getOperand(0).isRenamable();
-  bool SrcRenamable = false;
-  unsigned Num = 0;
-
-  for (RISCVMatInt::Inst &Inst : Seq) {
-    bool LastItem = ++Num == Seq.size();
-    unsigned DstRegState = getDeadRegState(DstIsDead && LastItem) |
-                           getRenamableRegState(Renamable);
-    unsigned SrcRegState = getKillRegState(SrcReg != RISCV::X0) |
-                           getRenamableRegState(SrcRenamable);
-    switch (Inst.getOpndKind()) {
-    case RISCVMatInt::Imm:
-      BuildMI(MBB, MBBI, DL, TII->get(Inst.getOpcode()))
-          .addReg(DstReg, RegState::Define | DstRegState)
-          .addImm(Inst.getImm());
-      break;
-    case RISCVMatInt::RegX0:
-      BuildMI(MBB, MBBI, DL, TII->get(Inst.getOpcode()))
-          .addReg(DstReg, RegState::Define | DstRegState)
-          .addReg(SrcReg, SrcRegState)
-          .addReg(RISCV::X0);
-      break;
-    case RISCVMatInt::RegReg:
-      BuildMI(MBB, MBBI, DL, TII->get(Inst.getOpcode()))
-          .addReg(DstReg, RegState::Define | DstRegState)
-          .addReg(SrcReg, SrcRegState)
-          .addReg(SrcReg, SrcRegState);
-      break;
-    case RISCVMatInt::RegImm:
-      BuildMI(MBB, MBBI, DL, TII->get(Inst.getOpcode()))
-          .addReg(DstReg, RegState::Define | DstRegState)
-          .addReg(SrcReg, SrcRegState)
-          .addImm(Inst.getImm());
-      break;
-    }
-    // Only the first instruction has X0 as its source.
-    SrcReg = DstReg;
-    SrcRenamable = Renamable;
-  }
+
+  TII->movImm(MBB, MBBI, DL, DstReg, Val, MachineInstr::NoFlags, Renamable,
+              DstIsDead);
+
   MBBI->eraseFromParent();
   return true;
 }



More information about the llvm-commits mailing list