[PATCH] D74306: [MIR][ARM] WIP: Print condition code names instead of magic constants

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 10 08:43:11 PST 2020


SjoerdMeijer added a comment.

In D74306#1867215 <https://reviews.llvm.org/D74306#1867215>, @efriedma wrote:

> I think something like this is nice to have: various instructions use immediates with weird encodings that make perfect sense in the code, but are sort of hard to read.


Thanks for taking a look!

> I don't really like the way this is implemented; it's adding a bunch of target-specific logic to target-independent code.

I could have tried harder to hide target specific logic in targets hooks in MIParse. In the MIRPrinter for example, I added a call to `TII->getPredicateByName()`. In the parser, for this WIP demo, I was lazy and just added `getPredicateCondCode()`, but could have done something similar. But I do fully agree of course that one way or another, that means more target specific parsing.

> I wonder if, instead of replacing the immediate with a keyword, it would make sense to add some generic mechanism to print a comments after an operand, to sort of explain it.  We could print the condition for a condition code, but we could also decode various other kinds of encoded immediates: for example, "26" is "lsl #3" in ADDrsi.  Printing it as a comment would be simpler to implement because the parsing change would be much smaller: we continue just parsing an integer, and ignore the comment, instead of adding target-specific parsing rules.

Nice one, cheers, I do really like that idea! The only disadvantage I see is that one can argue that MIR output is possibly more verbose than needed, but all together this sounds like an excellent trade-off to me. I will give this a try and make a start with implementing this soon.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74306/new/

https://reviews.llvm.org/D74306





More information about the llvm-commits mailing list