[llvm] a41b149 - [MachineInstr] add insert method for variadic instructions (#67699)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 30 15:00:03 PDT 2023
Author: Nick Desaulniers
Date: 2023-10-30T14:59:58-07:00
New Revision: a41b149f481e2bcba24e81f208a1938247f040e0
URL: https://github.com/llvm/llvm-project/commit/a41b149f481e2bcba24e81f208a1938247f040e0
DIFF: https://github.com/llvm/llvm-project/commit/a41b149f481e2bcba24e81f208a1938247f040e0.diff
LOG: [MachineInstr] add insert method for variadic instructions (#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). These are not 1:1 operand
replacements, but N:M operand replacements. i.e. we need to
update 2 MachineOperands into the middle of the operand list to 5 (at
least for x86_64).
- 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's NumOperands 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
This is meant to mirror `MCInst::insert`.
Added:
Modified:
llvm/include/llvm/CodeGen/MachineInstr.h
llvm/lib/CodeGen/MachineInstr.cpp
llvm/unittests/CodeGen/MachineInstrTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 0b9ad764af265de..4877f43e8578d1c 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -1812,6 +1812,9 @@ class MachineInstr
/// preferred.
void addOperand(const MachineOperand &Op);
+ /// Inserts Ops BEFORE It. Can untie/retie tied operands.
+ void insert(mop_iterator InsertBefore, ArrayRef<MachineOperand> Ops);
+
/// Replace the instruction descriptor (thus opcode) of
/// the current instruction with a new one.
void setDesc(const MCInstrDesc &TID);
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 448725893bde020..048563cc2bcc4e4 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -2481,3 +2481,48 @@ MachineInstr::getFirst5RegLLTs() const {
Reg2, getRegInfo()->getType(Reg2), Reg3, getRegInfo()->getType(Reg3),
Reg4, getRegInfo()->getType(Reg4));
}
+
+void MachineInstr::insert(mop_iterator InsertBefore,
+ ArrayRef<MachineOperand> Ops) {
+ assert(InsertBefore != nullptr && "invalid iterator");
+ assert(InsertBefore->getParent() == this &&
+ "iterator points to operand of other inst");
+ if (Ops.empty())
+ return;
+
+ // Do one pass to untie operands.
+ SmallDenseMap<unsigned, unsigned> TiedOpIndices;
+ for (const MachineOperand &MO : operands()) {
+ if (MO.isReg() && MO.isTied()) {
+ unsigned OpNo = getOperandNo(&MO);
+ unsigned TiedTo = findTiedOperandIdx(OpNo);
+ TiedOpIndices[OpNo] = TiedTo;
+ untieRegOperand(OpNo);
+ }
+ }
+
+ unsigned OpIdx = getOperandNo(InsertBefore);
+ unsigned NumOperands = getNumOperands();
+ 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);
+
+ // Re-tie operands.
+ for (auto [Tie1, Tie2] : TiedOpIndices) {
+ if (Tie1 >= OpIdx)
+ Tie1 += Ops.size();
+ if (Tie2 >= OpIdx)
+ Tie2 += Ops.size();
+ tieOperands(Tie1, Tie2);
+ }
+}
diff --git a/llvm/unittests/CodeGen/MachineInstrTest.cpp b/llvm/unittests/CodeGen/MachineInstrTest.cpp
index be409a56adb1af7..0841cd3a7fb04f3 100644
--- a/llvm/unittests/CodeGen/MachineInstrTest.cpp
+++ b/llvm/unittests/CodeGen/MachineInstrTest.cpp
@@ -478,4 +478,87 @@ 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());
+
+ // test tied operands
+ MCRegisterClass MRC{0, 0, 0, 0, 0, 0, 0, 0, /*Allocatable=*/true};
+ TargetRegisterClass RC{&MRC, 0, 0, {}, 0, 0, 0, 0, 0, 0, 0};
+ // MachineRegisterInfo will be very upset if these registers aren't
+ // allocatable.
+ assert(RC.isAllocatable() && "unusable TargetRegisterClass");
+ MachineRegisterInfo &MRI = MF->getRegInfo();
+ Register A = MRI.createVirtualRegister(&RC);
+ Register B = MRI.createVirtualRegister(&RC);
+ MI->getOperand(0).ChangeToRegister(A, /*isDef=*/true);
+ MI->getOperand(1).ChangeToRegister(B, /*isDef=*/false);
+ MI->tieOperands(0, 1);
+ EXPECT_TRUE(MI->getOperand(0).isTied());
+ EXPECT_TRUE(MI->getOperand(1).isTied());
+ EXPECT_EQ(MI->findTiedOperandIdx(0), 1U);
+ EXPECT_EQ(MI->findTiedOperandIdx(1), 0U);
+ MI->insert(&MI->getOperand(1), {MachineOperand::CreateImm(7)});
+ EXPECT_TRUE(MI->getOperand(0).isTied());
+ EXPECT_TRUE(MI->getOperand(1).isImm());
+ EXPECT_TRUE(MI->getOperand(2).isTied());
+ EXPECT_EQ(MI->findTiedOperandIdx(0), 2U);
+ EXPECT_EQ(MI->findTiedOperandIdx(2), 0U);
+ EXPECT_EQ(MI->getOperand(0).getReg(), A);
+ EXPECT_EQ(MI->getOperand(2).getReg(), B);
+
+ // bad inputs
+ EXPECT_EQ(MI->getNumOperands(), 10U);
+ MI->insert(MI->operands_begin(), {});
+ EXPECT_EQ(MI->getNumOperands(), 10U);
+}
+
} // end namespace
More information about the llvm-commits
mailing list