[PATCH] D102052: [MC][RISCV] Add RISCV MCObjectFileInfo

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 25 10:40:59 PDT 2021


MaskRay added inline comments.


================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:965
 
+MCObjectFileInfo::~MCObjectFileInfo() = default;
+
----------------
`{}`

`= default` doesn't change the fact that it is user-defined. IMHO `{}` is the prevailing style.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCObjectFileInfo.cpp:21
+  MCContext &Ctx = getContext();
+  const MCSubtargetInfo *STI = Ctx.getSubtargetInfo();
+  if (STI->hasFeature(RISCV::FeatureStdExtC)) {
----------------
Use just one variable. Don't keep `Ctx`


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCObjectFileInfo.cpp:23
+  if (STI->hasFeature(RISCV::FeatureStdExtC)) {
+    return 2;
+  }
----------------
Drop braces around simple statements.

Just use `? : `


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCObjectFileInfo.h:19
+namespace llvm {
+class Triple;
+
----------------
Is this used?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102052



More information about the llvm-commits mailing list