[llvm] [MachineInstr] add insert method for variadic instructions (PR #67699)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 5 07:32:21 PDT 2023
qcolombet wrote:
> 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?
The short answer is:
`MCInstrDesc` holds what is known statically.
`NumOperands` holds how many actual operands there are attached to this particular `MachineInstr` at compile time.
E.g., consider a `PHI` instruction.
Its `MCInstrDesc` is going to describe the number of operands we **must** have and that's `1` (the definition of the `PHI`).
Now the `NumOperands` of the related instantiation of a `PHI` will report the definition and all the arguments.
Generally speaking, there is a guarantee that `MI`'s `NumOperands` >= `MCInstrDesc`'s `NumOperands`.
(See https://github.com/llvm/llvm-project/blob/32d16b64d3125e76f65d7d88a302a33618eb0e6e/llvm/lib/CodeGen/MachineVerifier.cpp#L1828)
> How many operands can a MachineInstr have?
Unlimited :).
> Do I
> need to update BOTH (keeping them in sync)?"
`MCInstrDesc`'s `NumOperands` cannot be changed, so if you need to keep something in sync that means you need to set a different `MCInstrDesc` on the related `MI`. (E.g., go from `ADDI32rr` to `ADDI32rm`.)
`MI`'s `NumOperands` is updated automatically through the calls to `removeOperand` and `addOperand`.
> 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.
Hmm that doesn't sound right.
Where do you see this assert?
IIRC the only thing we check is that the number of definitions and the number of arguments are at least as big as what `MCInstrDesc` requires.
Then, of course, there's a bunch of other checks around register classes and whatnot, but with respect to the number of operands, that's about it I think.
Long story short, for inline asm, I don't think you have to worry about `NumOperands`.
https://github.com/llvm/llvm-project/pull/67699
More information about the llvm-commits
mailing list