[llvm] [MachineInstr] add insert method for variadic instructions (PR #67699)

Nick Desaulniers via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 30 09:55:04 PDT 2023


================
@@ -2475,3 +2475,45 @@ MachineInstr::getFirst5RegLLTs() const {
       Reg2, getRegInfo()->getType(Reg2), Reg3, getRegInfo()->getType(Reg3),
       Reg4, getRegInfo()->getType(Reg4));
 }
+
+void MachineInstr::insert(mop_iterator It, ArrayRef<MachineOperand> Ops) {
+  if (!It || Ops.empty())
+    return;
+
+  // Do one pass to untie operands.
+  DenseMap<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);
+    }
+  }
+
+  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);
----------------
nickdesaulniers wrote:

> On that note I wonder if moveOperands() would work here as well instead of the removeOperand / addOperand for MovingOps here?

The issue there is we would miss the calls to `MachineFunction::allocateOperandArray` from `MachineInstr::addOperand`. Since splice grows the operand list, we need to realloc the storage for the operands.  `MachineInstr::moveOperands` just `memmove`s operands around which is not safe to do when the backing store hasn't been grown.

> Admittedly it looks like moveOperands() doesn't really adjust the TiedTo markers so that's why it wouldn't work here?

That's also an issue.

> Maybe do this in reverse order so we waste less cycles on the moveOperands() call within MachineInstr::removeOperand().

I don't think there are any cycles to be saved? Perhaps if splice is being used to append operands on the end?  Usually you splice operands into the middle, so you need to memmove the elements occurring after the insertion point multiple times.

Notice in this loop that the induction variable `I` is not being used as an index; we're removing `OpIdx` repeatedly as this is slicing off the elements past the insertion point.

Later, we need to re-add the operands back in the correct order (though it's not hard to reverse the loop below).

Did I perhaps miss something thought?

https://github.com/llvm/llvm-project/pull/67699


More information about the llvm-commits mailing list