[PATCH] D116719: [NFC][RISCV] Make the macro names more uniform

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

Thanks for the patch. I wouldn't mark this as NFC, because the DEBUG_TYPE changes will impact backend developers using -debug-only to control debug messages. It would be worth noting this impact in the commit message too.

I've gone through and I think I'm slightly against most of the proposed DEBUG_TYPE changes. It doesn't always make sense to copy what other targets do (after all, one hope with the RISC-V backend was to try to avoid that copy and paste approach), but I think it's probably advantageous that the same format of -debug-only invocations is common across backends, even if the names themselves aren't always the best (or consistent in style).



================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp:28
 
-#define DEBUG_TYPE "riscvmcexpr"
+#define DEBUG_TYPE "riscv-mc-expr"
 
----------------
I was mirroring other targets with this one (e.g. Sparc, Mips, Lanai). But I don't think this is used enough that it's necessary to follow these. I'd suggest riscv-mcexpr is a better change though (and also matches HexagonMCExpr).


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:28
 
-#define DEBUG_TYPE "riscv-isel"
+#define DEBUG_TYPE "riscv-isel-dag-to-dag"
 
----------------
Basically all other archs use $target-isel for this, so it would be best to keep it the same.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:43
 
-#define DEBUG_TYPE "riscv-lower"
+#define DEBUG_TYPE "riscv-isel-lowering"
 
----------------
This is another one that matches what almost all other targets do (a few call it $target-isel instead), so would be best left unchanged.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:22
 
-#define DEBUG_TYPE "riscv-isel"
+#define DEBUG_TYPE "riscv-instruction-selector"
 
----------------
This also matches other targets (AArch64, AMDGPU, Arm, M68k, Mips).


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:16
 
-#define DEBUG_TYPE "riscvtti"
+#define DEBUG_TYPE "riscv-target-transform-info"
 
----------------
Again, the current name matches the format used for other backends.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116719