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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:87
+  setAttributeItems(Attribute, IntValue, StringValue,
+                    /* OverwriteExisting=*/true);
+}
----------------
`/* ` -> `/*`


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h:25
+      NumericAndTextAttributes
+    } Type;
+    unsigned Tag;
----------------
HsiangKai wrote:
> MaskRay wrote:
> > omit `Type`
> `Type` is a data member of `AttributeItem`.
It is not common to use `enum { ... } Type` in C++. `Type` is not referenced anyway.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h:56
+    Contents.push_back({AttributeItem::NumericAttribute, Attribute, Value,
+                        std::string(StringRef(""))});
+  }
----------------
`""` is sufficient


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023