[PATCH] D27984: [ARM] [RFC] Use AddDefaultCC/Pred where possible. NFC

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 10 09:46:34 PST 2017


I like this solution for being technically simple and still resulting in code that is easy to understand.

- I would return an std::array<MachineOperand, 2> from predOps()

We can do this after deciding on the style:
- I still think using a shorter name like MachineInstrBuilder::add() is a good idea here without loosing meaning (there is nothing except operands you can add in a MachineInstrBuilder)
- There's a number of places that look like this after the transformation that can now be simplified by concatenating function calls:
MIB.addReg(SrcReg, getKillRegState(KillSrc));
MIB.addOperands(predOps(ARMCC::AL));
MIB.addReg(ARM::CPSR, RegState::Implicit | RegState::Define);

- Matthias

> On Jan 10, 2017, at 3:18 AM, Diana Picus via Phabricator via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> rovka added a comment.
> 
> Hi,
> 
> Thanks to everyone for the suggestions.
> Would something like this [1] do the trick? I haven't used a pair because from the MachineInstrBuilder's POV there's nothing special about pairs of MachineOperands. I also replaced all uses here [2] - I did this automatically with a custom tool, so I can easily change all uses again if people think it's still ugly.
> 
> [1] https://reviews.llvm.org/differential/diff/83787/
> [2] https://reviews.llvm.org/differential/diff/83788/
> 
> 
> https://reviews.llvm.org/D27984
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list