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

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:102
+
+    Streamer.emitIntValue(ELFAttrs::Format_Version, 1);
+  }
----------------
`emitInt8`


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:113
+
+  Streamer.emitIntValue(VendorHeaderSize + TagHeaderSize + ContentsSize, 4);
+  Streamer.emitBytes(CurrentVendor);
----------------
emitInt32


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:115
+  Streamer.emitBytes(CurrentVendor);
+  Streamer.emitIntValue(0, 1); // '\0'
+
----------------
emitInt8


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:42
+    Arch = (Twine(Arch) + "_m2p0").str();
+
+  if (STI.hasFeature(RISCV::FeatureStdExtA))
----------------
These empty lines seem unneeded.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:44
+  if (STI.hasFeature(RISCV::FeatureStdExtA))
+    Arch = (Twine(Arch) + "_a2p0").str();
+
----------------
Just `Arch += "_a2p0";`


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:93
+                                               StringRef String) {
+  OS << "\t.attribute\t" << Attribute << ", \"" << String << "\"\n";
+}
----------------
Does `String` need to be escaped?


================
Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:186
+void RISCVAsmPrinter::emitEndOfAsmFile(Module &M) {
+  MCTargetStreamer &TS = *OutStreamer->getTargetStreamer();
+  RISCVTargetStreamer &RTS = static_cast<RISCVTargetStreamer &>(TS);
----------------
`TS` can be inlined.


================
Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:195
+  MCTargetStreamer &TS = *OutStreamer->getTargetStreamer();
+  RISCVTargetStreamer &RTS = static_cast<RISCVTargetStreamer &>(TS);
+
----------------
TS can be inlined.


================
Comment at: llvm/test/CodeGen/RISCV/attributes.ll:25
+
+define i32 @addi(i32 %a) nounwind {
+  %1 = add i32 %a, 1
----------------
Delete `nounwind` (unneeded detail)


================
Comment at: llvm/test/CodeGen/RISCV/attributes.ll:29
+}
+
----------------
Delete empty line


================
Comment at: llvm/test/MC/RISCV/attribute-arch.s:38
+# CHECK: attribute      5, "rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
+
----------------
Delete empty line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023