[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
Thu Dec 5 21:01:15 PST 2019


jsji added a comment.

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

> I might object less to a _rec suffix.  However, you would still need to understand what "record form" means, which requires understanding of the ISA.  Why not simply add a comment block to the top of PPCInstrInfo.td, describing things like this?  I have a feeling other newbies would have similar questions/concerns/complaints, and they can't all be addressed by renaming everything.


Although I still prefer `_record` as what Hal proposed. Verbose and Simplicity. `_rec` also acceptable to me.

> I would propose something along the lines of the following as a block comment in PPCInstrInfo.td:

Sure, we can always add similar comments besides renaming.

The downside of only putting some block comment in certain file is:
most of the time you will still miss it if you don't happen to look into the specific section of that file.

I don't expect everyone will able to find and read this specific block comment in `PPCInstrInfo.td`,
when he/she start to look at some code, and confused about `SUBICo` and `SUBIC`.
He/she may be able to look back into definition of `SUBIC`, but if the comment is not nearby,
he might not have idea to where to find that block of comments.
And we can't expect everyone to be able to read all important files, or know which file to look at.


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