[PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

The change to ARMAttributeParser.cpp (`Error (ARMAttributeParser::*routine)(AttrType);` -> `Error (ARMAttributeParser::*routine)(unsigned);`) is not needed.

Some common parsing tests can be moved from `unittests/Support/ARMAttributeParser.cpp` to `unittests/Object/ELFAttributeParser.cpp`

I have checked whether it is easy to move `Support/ARM{AttributeParser,BuildAttrs,TargetParser}.cpp` to `Object/`. It is not, but I think the end goal is probably to have them under `Object/`.

The llvm-readobj changes and tests can be moved to a separate patch.



================
Comment at: llvm/include/llvm/Support/ARMAttributeParser.h:26
     ARMBuildAttrs::AttrType attribute;
-    Error (ARMAttributeParser::*routine)(ARMBuildAttrs::AttrType);
+    Error (ARMAttributeParser::*routine)(unsigned);
   };
----------------
It is not necessary to change `AttrType` to unsigned.

Reverting the change the reduce the diff in this file to a minimum.


================
Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:183
+
+  if (TT.isOSBinFormatELF())
+    emitAttributes();
----------------
The variable can be inlined


================
Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:192
+
+  if (TT.isOSBinFormatELF()) {
+    RTS.finishAttributeSection();
----------------
excess braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023