[llvm-dev] MachineIRBuilder API

[Arsenault, Matthew via llvm-dev]

Hi,

I’m finding the API for MachineIRBuilder to be pretty annoying to use compared to SelectionDAG. I think it’s making the majority of code more verbose and unwieldly for anything less than trivial. I think changing it to behave more like the DAG.get* functions would make it easier to use, and decrease the mental overhead when porting code from SelectionDAG.

The main issue is needing to create and/or manage the registers used separately from the instruction creation. Normally you have to create a generic virtual register with the correct output type. When a lot of intermediate values are needed, this can get pretty messy. There is an alternative provided, but I don’t think it’s really any better. You can construct a DstOp from an LLT, which will create a new register. You would then need to use the returned instruction, get the destination register from the output and use that. This doesn’t allow you to naturally compose operations by using the return value of a build* function to another. I also find this aspect to be pretty error prone, as I discovered it by accident when the registers I had already created weren’t being used.

The current set of build* functions all return MachineInstrBuilder, or effectively the inserted instruction. This seems reasonable for some odd low level cases where you are building an instruction that won’t be inserted yet, or you don’t know the operands ahead of time. For the majority of code, I don’t see how this is useful. Most places should be using a buildFooInstr type of instruction, which creates and inserts a specific opcode (or set of similar behaving opcodes) with all of the necessary operands. The instruction is already inserted, and can’t have additional operands added, so there isn’t really anything to do with it.

It would be more helpful if these were returning the register / logical value, similar to what you get with DAG.getNode(). In the odd case you actually want the instruction, you could check the instruction before the insert point.

Does anyone feel strongly that these functions should be returning the builder? What I would like to do is either replace and complete the current set of buildOpcode functions with ones which return the output register value, or add an alternative set which do. Is it worth having two sets of functions (a buildFoo and getFoo set?). The same issue applies with opcodes producing multiple results, such as buildUnmerge, but in that case I would probably change these to a SmallVectorImpl out argument.

Another minor issue I’ve run into is when trying to add convenience functions with constants (e.g. shift creation with materializing the amount constant). With an overload using a SrcOp constructible from “unsigned”, and one using an integer type, the wrong one gets called and ends up creating a constant with the value of the register. I think this is more a symptom of using a plain unsigned for registers, but fixing this longstanding mistake everywhere would be a very large project.

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190131/b4ff6090/attachment.html>