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

Jinsong Ji via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 31 12:05:56 PST 2019


jsji added a comment.

> {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings <http://results.llvm-merge-guard.org/amd64_debian_testing_clang8-727/clang-tidy.txt>.



> ...llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7399:28: warning: invalid case style for function 'LowerTRUNCATEVector' [readability-identifier-naming]
> SDValue PPCTargetLowering::LowerTRUNCATEVector(SDValue Op,
> 
>   ^~~~~~~~~~~~~~~~~~~
>    lowerTruncateVector

I have fixed one the the clang-tidy failure in https://github.com/llvm/llvm-project/commit/fcbf05bbdccc8a32f6a80316ea1c13be7e7eeae2, 
but as @nemanjai pointed out 'It is questionable how/whether this renaming applies to well established conventions that do not conform…'

In this case it might be worse, as we have some non-conform naming in target independent code as well.

eg:

> $ grep ":Lower.*(" llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
> const char *TargetLowering::LowerXConstraint(EVT ConstraintVT) const {
> SDValue TargetLowering::LowerAsmOutputForConstraint(
> void TargetLowering::LowerAsmOperandForConstraint(SDValue Op,
> SDValue TargetLowering::LowerToTLSEmulatedModel(const GlobalAddressSDNode *GA,

My understanding is **we want to avoid large-scale renaming just due to naming convention, but we should gradually transform to keep the source code clang-tidy conform, and pre-merge testing green**?
In some cases, this means we will gradually getting some new code that **conform to clang-tidy, but break well established conventions **.

Is that OK? @hfinkel 
If not, what direction we should pursue?  Thanks.


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