[PATCH] D70758: [PowerPC][NFC] Rename record instructions to use _record suffix instead of o

Jinsong Ji via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 3 12:16:22 PST 2019


jsji added a comment.

In D70758#1767545 <https://reviews.llvm.org/D70758#1767545>, @jhibbits wrote:

> I personally think the status quo is easy to read.  The definitions look like the instructions themselves, and the added verbosity seems very unnecessary.  I don't know about others, but I consider people working in the PPCInstr*.td files to be either knowing exactly what they're looking for (grepping), or have a sufficient handle on the ISA that the existing notation is clear.  At most I could see changing the 'o' suffix to '_o', which might help newcomers familiar with '_' notation to indicate a diminutive or subscript, but '_record' is very verbose, and as @nemanjai points out, can make some text very ugly.


'status quo' was OK if we don't have XO-form support.

The motivation of this patch is to avoid confusion between 'o' and 'O' suffixes, and make it easier to connect 'o' to record form in ISA, 
apparently,  changing 'o' to '_o' won't help with that.

I see the benefit of verbose (_record) here: **lower the barrier of 2-3 levels for newcomers.**

1. You don't necessarily know 'o' at the end of opcode should be read as 'dot', not as o character itself.
2. You don't necessarily know that 'o' will be corresponding to period(.)  at the end of assembly mnemonics.
3. You don't necessarily know that mnemonics with period(.) or dot suffixes is called **record form** in ISA.
4. You don't necessarily know that **record form** will modify condition registers

It will take a newcomer at least above 4 steps to figure out what is the difference between `SUBICo` and `SUBIC`,
and what is worse, one may easily be blocked by misunderstandings in any of these steps.

But with `SUBIC_record`, one can easily figure out it in one step: searching SUBIC and record in ISA.

Yes, `_record` is a bit longer and verbose, and may be somehow *ugly* to someone, 
but I see the benefits outweigh tidy/compact code.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70758





More information about the llvm-commits mailing list