[llvm-dev] MachineIRBuilder API

Wed Jan 30 19:21:02 PST 2019

This is already possible.
To do the equivalent of what you mentioned, this should do it.
auto Add = Builder.buildAdd(LLT::scalar(32), Src1, Src);
auto Mul = Builder.buildMul(LLT::scalar(32), Add, Src3);

On Wed, Jan 30, 2019 at 6:57 PM Amara Emerson via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> I agree that it’s annoying to have to create VRegs manually. I haven’t
> thought this through deeply but perhaps having a utility wrapper class to
> capture the destination regs of a newly built instruction could help, e.g.
> something like:
> InstReg AddReg(MIRBuilder.buildAdd(LLT::scalar(32), Src1, Src));
> InstReg MulReg(MIRBuilder.buildMul(LLT::scalar(32), AddReg.get(), Src3));
> // etc etc
>
> Where InstReg could be something like:
> class InstReg {
>   unsigned Reg = 0;
> public:
>   InstReg(MachineInstrBuilder &MIB) :
> Reg(MIB.getInstr().getOperand(0).getReg()) {}
>   unsigned getReg() { return Reg; }
> };
>
> This example only works for single dest reg instructions but could be
> extended for any number of regs.
>
> Amara
>
> On 30 Jan 2019, at 18:43, Arsenault, Matthew via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> I’m less interested in literally passing the output of one as an argument
> to the next such that argument evaluation order matters. Intermediate
> register variables are fine. It would still be less code to have tmp =
> buildFoo(), buildFoo(tmp) than the current code required to manage the
> register
>
> -Matt
>
> *From: *<daniel_l_sanders at apple.com> on behalf of Daniel Sanders <
> daniel_l_sanders at apple.com>
> *Date: *Wednesday, January 30, 2019 at 9:32 PM
> *To: *"Arsenault, Matthew" <Matthew.Arsenault at amd.com>
> *Cc: *llvm-dev <llvm-dev at lists.llvm.org>
> *Subject: *Re: [llvm-dev] MachineIRBuilder API
>
> Hi Matt,
>
> Personally, I like that it follows the same pattern as MachineInstrBuilder
> used elsewhere in MIR. It would certainly be nice to create whole
> expressions at once though.
>
> I believe that nested SelectionDAG-style get*() functions have an ordering
> problem when used for MIR that wasn't a problem for SelectionDAG due to the
> difference in underlying representation. The DAG didn't really care which
> evaluation order happened as the de-duplication and unordered nature of the
> DAG produced the same result either way. For MIR, each builder needs to
> insert into a linear list so the evaluation order of those insertions
> matters. For example:
> getAdd(getSub(a, b), getMul(c, d))
> could produce either:
> %0 = G_SUB %a, %b
> %1 = G_MUL %c, %d
> %2 = G_ADD %0, %1
> or:
> %0 = G_MUL %c, %d
> %1 = G_SUB %a, %b
> %2 = G_ADD %1, %0
> depending on the (indeterminate) evaluation order of the arguments to
> getAdd(). The G_ADD is always last because getSub() and getMul() must
> execute before getAdd() but AFAIK there's nothing that controls the
> relative order that getSub() and getMul() are evaluated and inserted beyond
> saying that they aren't interleaved. If I'm interpreting
> https://en.cppreference.com/w/cpp/language/eval_order correctly, C++17
> makes it more explicit than the old rules (rule 15 in particular).
>
>
> On Jan 30, 2019, at 16:55, Arsenault, Matthew via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> 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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

-- 
Regards,
Aditya
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190130/1cb4f158/attachment-0001.html>