[llvm] da695de - [MachineInstrBuilder] Introduce MIMetadata to simplify metadata propagation
Marco Elver via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 7 02:24:15 PDT 2022
Author: Marco Elver
Date: 2022-09-07T11:22:50+02:00
New Revision: da695de628d6507924f49b8088c812eeb908b2ee
URL: https://github.com/llvm/llvm-project/commit/da695de628d6507924f49b8088c812eeb908b2ee
DIFF: https://github.com/llvm/llvm-project/commit/da695de628d6507924f49b8088c812eeb908b2ee.diff
LOG: [MachineInstrBuilder] Introduce MIMetadata to simplify metadata propagation
In many places DebugLoc and PCSections metadata are just copied along to
propagate them through MachineInstrs. Simplify doing so by bundling them
up in a MIMetadata class that replaces the DebugLoc argument to most
BuildMI() variants.
The DebugLoc-only constructors allow implicit construction, so that
existing usage of `BuildMI(.., DL, ..)` works as before, and the rest of
the codebase using BuildMI() does not require changes.
NFC.
Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D130883
Added:
Modified:
llvm/include/llvm/CodeGen/MachineInstrBuilder.h
llvm/unittests/CodeGen/MachineInstrTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MachineInstrBuilder.h b/llvm/include/llvm/CodeGen/MachineInstrBuilder.h
index 80f30231aef2f..7af53cbb43bb0 100644
--- a/llvm/include/llvm/CodeGen/MachineInstrBuilder.h
+++ b/llvm/include/llvm/CodeGen/MachineInstrBuilder.h
@@ -310,6 +310,12 @@ class MachineInstrBuilder {
}
}
+ const MachineInstrBuilder &setPCSections(MDNode *MD) const {
+ if (MD)
+ MI->setPCSections(*MF, MD);
+ return *this;
+ }
+
/// Copy all the implicit operands from OtherMI onto this one.
const MachineInstrBuilder &
copyImplicitOps(const MachineInstr &OtherMI) const {
@@ -324,17 +330,42 @@ class MachineInstrBuilder {
}
};
+/// Set of metadata that should be preserved when using BuildMI(). This provides
+/// a more convenient way of preserving DebugLoc and PCSections.
+class MIMetadata {
+public:
+ MIMetadata() = default;
+ MIMetadata(DebugLoc DL, MDNode *PCSections = nullptr)
+ : DL(std::move(DL)), PCSections(PCSections) {}
+ MIMetadata(const DILocation *DI, MDNode *PCSections = nullptr)
+ : DL(DI), PCSections(PCSections) {}
+ explicit MIMetadata(const Instruction &From)
+ : DL(From.getDebugLoc()),
+ PCSections(From.getMetadata(LLVMContext::MD_pcsections)) {}
+ explicit MIMetadata(const MachineInstr &From)
+ : DL(From.getDebugLoc()), PCSections(From.getPCSections()) {}
+
+ const DebugLoc &getDL() const { return DL; }
+ MDNode *getPCSections() const { return PCSections; }
+
+private:
+ DebugLoc DL;
+ MDNode *PCSections = nullptr;
+};
+
/// Builder interface. Specify how to create the initial instruction itself.
-inline MachineInstrBuilder BuildMI(MachineFunction &MF, const DebugLoc &DL,
+inline MachineInstrBuilder BuildMI(MachineFunction &MF, const MIMetadata &MIMD,
const MCInstrDesc &MCID) {
- return MachineInstrBuilder(MF, MF.CreateMachineInstr(MCID, DL));
+ return MachineInstrBuilder(MF, MF.CreateMachineInstr(MCID, MIMD.getDL()))
+ .setPCSections(MIMD.getPCSections());
}
/// This version of the builder sets up the first operand as a
/// destination virtual register.
-inline MachineInstrBuilder BuildMI(MachineFunction &MF, const DebugLoc &DL,
+inline MachineInstrBuilder BuildMI(MachineFunction &MF, const MIMetadata &MIMD,
const MCInstrDesc &MCID, Register DestReg) {
- return MachineInstrBuilder(MF, MF.CreateMachineInstr(MCID, DL))
+ return MachineInstrBuilder(MF, MF.CreateMachineInstr(MCID, MIMD.getDL()))
+ .setPCSections(MIMD.getPCSections())
.addReg(DestReg, RegState::Define);
}
@@ -343,12 +374,14 @@ inline MachineInstrBuilder BuildMI(MachineFunction &MF, const DebugLoc &DL,
/// operand as a destination virtual register.
inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB,
MachineBasicBlock::iterator I,
- const DebugLoc &DL, const MCInstrDesc &MCID,
- Register DestReg) {
+ const MIMetadata &MIMD,
+ const MCInstrDesc &MCID, Register DestReg) {
MachineFunction &MF = *BB.getParent();
- MachineInstr *MI = MF.CreateMachineInstr(MCID, DL);
+ MachineInstr *MI = MF.CreateMachineInstr(MCID, MIMD.getDL());
BB.insert(I, MI);
- return MachineInstrBuilder(MF, MI).addReg(DestReg, RegState::Define);
+ return MachineInstrBuilder(MF, MI)
+ .setPCSections(MIMD.getPCSections())
+ .addReg(DestReg, RegState::Define);
}
/// This version of the builder inserts the newly-built instruction before
@@ -359,28 +392,31 @@ inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB,
/// added to the same bundle.
inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB,
MachineBasicBlock::instr_iterator I,
- const DebugLoc &DL, const MCInstrDesc &MCID,
- Register DestReg) {
+ const MIMetadata &MIMD,
+ const MCInstrDesc &MCID, Register DestReg) {
MachineFunction &MF = *BB.getParent();
- MachineInstr *MI = MF.CreateMachineInstr(MCID, DL);
+ MachineInstr *MI = MF.CreateMachineInstr(MCID, MIMD.getDL());
BB.insert(I, MI);
- return MachineInstrBuilder(MF, MI).addReg(DestReg, RegState::Define);
+ return MachineInstrBuilder(MF, MI)
+ .setPCSections(MIMD.getPCSections())
+ .addReg(DestReg, RegState::Define);
}
inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB, MachineInstr &I,
- const DebugLoc &DL, const MCInstrDesc &MCID,
- Register DestReg) {
+ const MIMetadata &MIMD,
+ const MCInstrDesc &MCID, Register DestReg) {
// Calling the overload for instr_iterator is always correct. However, the
// definition is not available in headers, so inline the check.
if (I.isInsideBundle())
- return BuildMI(BB, MachineBasicBlock::instr_iterator(I), DL, MCID, DestReg);
- return BuildMI(BB, MachineBasicBlock::iterator(I), DL, MCID, DestReg);
+ return BuildMI(BB, MachineBasicBlock::instr_iterator(I), MIMD, MCID,
+ DestReg);
+ return BuildMI(BB, MachineBasicBlock::iterator(I), MIMD, MCID, DestReg);
}
inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB, MachineInstr *I,
- const DebugLoc &DL, const MCInstrDesc &MCID,
- Register DestReg) {
- return BuildMI(BB, *I, DL, MCID, DestReg);
+ const MIMetadata &MIMD,
+ const MCInstrDesc &MCID, Register DestReg) {
+ return BuildMI(BB, *I, MIMD, MCID, DestReg);
}
/// This version of the builder inserts the newly-built instruction before the
@@ -388,53 +424,55 @@ inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB, MachineInstr *I,
/// destination register.
inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB,
MachineBasicBlock::iterator I,
- const DebugLoc &DL,
+ const MIMetadata &MIMD,
const MCInstrDesc &MCID) {
MachineFunction &MF = *BB.getParent();
- MachineInstr *MI = MF.CreateMachineInstr(MCID, DL);
+ MachineInstr *MI = MF.CreateMachineInstr(MCID, MIMD.getDL());
BB.insert(I, MI);
- return MachineInstrBuilder(MF, MI);
+ return MachineInstrBuilder(MF, MI).setPCSections(MIMD.getPCSections());
}
inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB,
MachineBasicBlock::instr_iterator I,
- const DebugLoc &DL,
+ const MIMetadata &MIMD,
const MCInstrDesc &MCID) {
MachineFunction &MF = *BB.getParent();
- MachineInstr *MI = MF.CreateMachineInstr(MCID, DL);
+ MachineInstr *MI = MF.CreateMachineInstr(MCID, MIMD.getDL());
BB.insert(I, MI);
- return MachineInstrBuilder(MF, MI);
+ return MachineInstrBuilder(MF, MI).setPCSections(MIMD.getPCSections());
}
inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB, MachineInstr &I,
- const DebugLoc &DL,
+ const MIMetadata &MIMD,
const MCInstrDesc &MCID) {
// Calling the overload for instr_iterator is always correct. However, the
// definition is not available in headers, so inline the check.
if (I.isInsideBundle())
- return BuildMI(BB, MachineBasicBlock::instr_iterator(I), DL, MCID);
- return BuildMI(BB, MachineBasicBlock::iterator(I), DL, MCID);
+ return BuildMI(BB, MachineBasicBlock::instr_iterator(I), MIMD, MCID);
+ return BuildMI(BB, MachineBasicBlock::iterator(I), MIMD, MCID);
}
inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB, MachineInstr *I,
- const DebugLoc &DL,
+ const MIMetadata &MIMD,
const MCInstrDesc &MCID) {
- return BuildMI(BB, *I, DL, MCID);
+ return BuildMI(BB, *I, MIMD, MCID);
}
/// This version of the builder inserts the newly-built instruction at the end
/// of the given MachineBasicBlock, and does NOT take a destination register.
-inline MachineInstrBuilder BuildMI(MachineBasicBlock *BB, const DebugLoc &DL,
+inline MachineInstrBuilder BuildMI(MachineBasicBlock *BB,
+ const MIMetadata &MIMD,
const MCInstrDesc &MCID) {
- return BuildMI(*BB, BB->end(), DL, MCID);
+ return BuildMI(*BB, BB->end(), MIMD, MCID);
}
/// This version of the builder inserts the newly-built instruction at the
/// end of the given MachineBasicBlock, and sets up the first operand as a
/// destination virtual register.
-inline MachineInstrBuilder BuildMI(MachineBasicBlock *BB, const DebugLoc &DL,
+inline MachineInstrBuilder BuildMI(MachineBasicBlock *BB,
+ const MIMetadata &MIMD,
const MCInstrDesc &MCID, Register DestReg) {
- return BuildMI(*BB, BB->end(), DL, MCID, DestReg);
+ return BuildMI(*BB, BB->end(), MIMD, MCID, DestReg);
}
/// This version of the builder builds a DBG_VALUE intrinsic
diff --git a/llvm/unittests/CodeGen/MachineInstrTest.cpp b/llvm/unittests/CodeGen/MachineInstrTest.cpp
index 555215182f0ba..6488f24e54a8a 100644
--- a/llvm/unittests/CodeGen/MachineInstrTest.cpp
+++ b/llvm/unittests/CodeGen/MachineInstrTest.cpp
@@ -10,6 +10,7 @@
#include "llvm/ADT/Triple.h"
#include "llvm/CodeGen/MachineBasicBlock.h"
#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineInstrBuilder.h"
#include "llvm/CodeGen/MachineMemOperand.h"
#include "llvm/CodeGen/MachineModuleInfo.h"
#include "llvm/CodeGen/TargetFrameLowering.h"
@@ -25,6 +26,7 @@
#include "llvm/Support/TargetSelect.h"
#include "llvm/Target/TargetMachine.h"
#include "llvm/Target/TargetOptions.h"
+#include "gmock/gmock.h"
#include "gtest/gtest.h"
using namespace llvm;
@@ -442,6 +444,34 @@ TEST(MachineInstrDebugValue, AddDebugValueOperand) {
}
}
+MATCHER_P(HasMIMetadata, MIMD, "") {
+ return arg->getDebugLoc() == MIMD.getDL() &&
+ arg->getPCSections() == MIMD.getPCSections();
+}
+
+TEST(MachineInstrBuilder, BuildMI) {
+ LLVMContext Ctx;
+ MDNode *PCS = MDNode::getDistinct(Ctx, None);
+ MDNode *DI = MDNode::getDistinct(Ctx, None);
+ DebugLoc DL(DI);
+ MIMetadata MIMD(DL, PCS);
+ EXPECT_EQ(MIMD.getDL(), DL);
+ EXPECT_EQ(MIMD.getPCSections(), PCS);
+ // Check common BuildMI() overloads propagate MIMetadata.
+ Module Mod("Module", Ctx);
+ auto MF = createMachineFunction(Ctx, Mod);
+ auto MBB = MF->CreateMachineBasicBlock();
+ MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, nullptr, nullptr, nullptr};
+ EXPECT_THAT(BuildMI(*MF, MIMD, MCID), HasMIMetadata(MIMD));
+ EXPECT_THAT(BuildMI(*MF, MIMD, MCID), HasMIMetadata(MIMD));
+ EXPECT_THAT(BuildMI(*MBB, MBB->end(), MIMD, MCID), HasMIMetadata(MIMD));
+ EXPECT_THAT(BuildMI(*MBB, MBB->end(), MIMD, MCID), HasMIMetadata(MIMD));
+ EXPECT_THAT(BuildMI(*MBB, MBB->instr_end(), MIMD, MCID), HasMIMetadata(MIMD));
+ EXPECT_THAT(BuildMI(*MBB, *MBB->begin(), MIMD, MCID), HasMIMetadata(MIMD));
+ EXPECT_THAT(BuildMI(*MBB, &*MBB->begin(), MIMD, MCID), HasMIMetadata(MIMD));
+ EXPECT_THAT(BuildMI(MBB, MIMD, MCID), HasMIMetadata(MIMD));
+}
+
static_assert(std::is_trivially_copyable<MCOperand>::value,
"trivially copyable");
More information about the llvm-commits
mailing list