[llvm] [MachineInstr] add insert method for variadic instructions (PR #67699)
Nick Desaulniers via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 28 09:33:52 PDT 2023
https://github.com/nickdesaulniers created https://github.com/llvm/llvm-project/pull/67699
As alluded to in #20571, it would be nice if we could mutate operand
lists of MachineInstr's more safely. Add an insert method that together
with removeOperand allows for easier splicing of operands.
Splitting this patch off early to get feedback; I need to either:
- mutate an INLINEASM{_BR} MachinInstr's MachineOperands from being
registers (physical or virtual) to memory
(MachineOperandType::MO_FrameIndex).
- copy, modify, write a new MachineInstr which has its relevant operands
replaced.
Either approaches are hazarded by existing references to either the
operands being moved, or the instruction being removed+replaced. For my
purposes in regalloc, either seem to work for me, so hopefully reviewers
can help me determine which approach is preferable. The second would
involve no new methods on MachineInstr.
One question I had while looking at this was: "why does MachineInstr
have BOTH a NumOperands member AND a MCInstrDesc member that itself has
a NumOperands member? How many operands can a MachineInstr have? Do I
need to update BOTH (keeping them in sync)?" FWICT, only "variadic"
MachineInstrs have MCInstrDesc with NumOperands (of the MCInstrDesc) set
to zero. If the MCInstrDesc is non-zero, then the NumOperands on the
MachineInstr itself cannot exceed this value (IIUC) else an assert will
be triggered.
For most non-psuedo instructions (or at least non-varidic instructions),
insert is less likely to be useful.
To run the newly added unittest:
$ pushd llvm/build; ninja CodeGenTests; popd
$ ./llvm/build/unittests/CodeGen/CodeGenTests \
--gtest_filter=MachineInstrTest.SpliceOperands
>From ff094aa51cf4778d79e49e14985a6565f3d653b6 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Wed, 27 Sep 2023 18:10:45 -0700
Subject: [PATCH] [MachineInstr] add insert method for variadic instructions
As alluded to in #20571, it would be nice if we could mutate operand
lists of MachineInstr's more safely. Add an insert method that together
with removeOperand allows for easier splicing of operands.
Splitting this patch off early to get feedback; I need to either:
- mutate an INLINEASM{_BR} MachinInstr's MachineOperands from being
registers (physical or virtual) to memory
(MachineOperandType::MO_FrameIndex).
- copy, modify, write a new MachineInstr which has its relevant operands
replaced.
Either approaches are hazarded by existing references to either the
operands being moved, or the instruction being removed+replaced. For my
purposes in regalloc, either seem to work for me, so hopefully reviewers
can help me determine which approach is preferable. The second would
involve no new methods on MachineInstr.
One question I had while looking at this was: "why does MachineInstr
have BOTH a NumOperands member AND a MCInstrDesc member that itself has
a NumOperands member? How many operands can a MachineInstr have? Do I
need to update BOTH (keeping them in sync)?" FWICT, only "variadic"
MachineInstrs have MCInstrDesc with NumOperands (of the MCInstrDesc) set
to zero. If the MCInstrDesc is non-zero, then the NumOperands on the
MachineInstr itself cannot exceed this value (IIUC) else an assert will
be triggered.
For most non-psuedo instructions (or at least non-varidic instructions),
insert is less likely to be useful.
To run the newly added unittest:
$ pushd llvm/build; ninja CodeGenTests; popd
$ ./llvm/build/unittests/CodeGen/CodeGenTests \
--gtest_filter=MachineInstrTest.SpliceOperands
---
llvm/include/llvm/CodeGen/MachineInstr.h | 2 +
llvm/lib/CodeGen/MachineInstr.cpp | 21 ++++++++
llvm/unittests/CodeGen/MachineInstrTest.cpp | 53 +++++++++++++++++++++
3 files changed, 76 insertions(+)
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 03fb15f77c65cbb..b3cd5573000a190 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -1818,6 +1818,8 @@ class MachineInstr
/// preferred.
void addOperand(const MachineOperand &Op);
+ void insert(mop_iterator It, ArrayRef<MachineOperand> Ops);
+
/// Replace the instruction descriptor (thus opcode) of
/// the current instruction with a new one.
void setDesc(const MCInstrDesc &TID) { MCID = &TID; }
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index d8467e2af8786ec..a3db83917466598 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -2475,3 +2475,24 @@ MachineInstr::getFirst5RegLLTs() const {
Reg2, getRegInfo()->getType(Reg2), Reg3, getRegInfo()->getType(Reg3),
Reg4, getRegInfo()->getType(Reg4));
}
+
+void MachineInstr::insert(mop_iterator It, ArrayRef<MachineOperand> Ops) {
+ assert(isVariadic() && "can only modify variadic instructions");
+ assert(It->getParent() == this && "iterator points to operand of other inst");
+
+ const unsigned OpIdx = getOperandNo(It);
+ const unsigned NumOperands = getNumOperands();
+ const unsigned OpsToMove = NumOperands - OpIdx;
+
+ SmallVector<MachineOperand> MovingOps;
+ MovingOps.reserve(OpsToMove);
+
+ for (unsigned I = 0; I < OpsToMove; ++I) {
+ MovingOps.emplace_back(getOperand(OpIdx));
+ removeOperand(OpIdx);
+ }
+ for (const MachineOperand &MO : Ops)
+ addOperand(MO);
+ for (const MachineOperand &OpMoved : MovingOps)
+ addOperand(OpMoved);
+}
diff --git a/llvm/unittests/CodeGen/MachineInstrTest.cpp b/llvm/unittests/CodeGen/MachineInstrTest.cpp
index be409a56adb1af7..9a225db34d6177e 100644
--- a/llvm/unittests/CodeGen/MachineInstrTest.cpp
+++ b/llvm/unittests/CodeGen/MachineInstrTest.cpp
@@ -478,4 +478,57 @@ TEST(MachineInstrBuilder, BuildMI) {
static_assert(std::is_trivially_copyable_v<MCOperand>, "trivially copyable");
+TEST(MachineInstrTest, SpliceOperands) {
+ LLVMContext Ctx;
+ Module Mod("Module", Ctx);
+ std::unique_ptr<MachineFunction> MF = createMachineFunction(Ctx, Mod);
+ MachineBasicBlock *MBB = MF->CreateMachineBasicBlock();
+ MCInstrDesc MCID = {TargetOpcode::INLINEASM,
+ 0,
+ 0,
+ 0,
+ 0,
+ 0,
+ 0,
+ 0,
+ 0,
+ (1ULL << MCID::Pseudo) | (1ULL << MCID::Variadic),
+ 0};
+ MachineInstr *MI = MF->CreateMachineInstr(MCID, DebugLoc());
+ MBB->insert(MBB->begin(), MI);
+ MI->addOperand(MachineOperand::CreateImm(0));
+ MI->addOperand(MachineOperand::CreateImm(1));
+ MI->addOperand(MachineOperand::CreateImm(2));
+ MI->addOperand(MachineOperand::CreateImm(3));
+ MI->addOperand(MachineOperand::CreateImm(4));
+
+ MI->removeOperand(1);
+ EXPECT_EQ(MI->getOperand(1).getImm(), MachineOperand::CreateImm(2).getImm());
+ EXPECT_EQ(MI->getNumOperands(), 4U);
+
+ MachineOperand Ops[] = {
+ MachineOperand::CreateImm(42), MachineOperand::CreateImm(1024),
+ MachineOperand::CreateImm(2048), MachineOperand::CreateImm(4096),
+ MachineOperand::CreateImm(8192),
+ };
+ auto *It = MI->operands_begin();
+ ++It;
+ MI->insert(It, Ops);
+
+ EXPECT_EQ(MI->getNumOperands(), 9U);
+ EXPECT_EQ(MI->getOperand(0).getImm(), MachineOperand::CreateImm(0).getImm());
+ EXPECT_EQ(MI->getOperand(1).getImm(), MachineOperand::CreateImm(42).getImm());
+ EXPECT_EQ(MI->getOperand(2).getImm(),
+ MachineOperand::CreateImm(1024).getImm());
+ EXPECT_EQ(MI->getOperand(3).getImm(),
+ MachineOperand::CreateImm(2048).getImm());
+ EXPECT_EQ(MI->getOperand(4).getImm(),
+ MachineOperand::CreateImm(4096).getImm());
+ EXPECT_EQ(MI->getOperand(5).getImm(),
+ MachineOperand::CreateImm(8192).getImm());
+ EXPECT_EQ(MI->getOperand(6).getImm(), MachineOperand::CreateImm(2).getImm());
+ EXPECT_EQ(MI->getOperand(7).getImm(), MachineOperand::CreateImm(3).getImm());
+ EXPECT_EQ(MI->getOperand(8).getImm(), MachineOperand::CreateImm(4).getImm());
+}
+
} // end namespace
More information about the llvm-commits
mailing list