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

Hsiangkai Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 24 02:07:56 PDT 2020


HsiangKai marked 3 inline comments as done.
HsiangKai added inline comments.


================
Comment at: llvm/include/llvm/Support/ARMBuildAttributes.h:37
   File                      = 1,
   CPU_raw_name              = 4,
   CPU_name                  = 5,
----------------
jhenderson wrote:
> Same comment as elsewhere.
I prefer to keep the formatting in this file. We could create another NFC patch for it.


================
Comment at: llvm/lib/Support/ARMBuildAttrs.cpp:15
   { ARMBuildAttrs::File, "Tag_File" },
   { ARMBuildAttrs::Section, "Tag_Section" },
   { ARMBuildAttrs::Symbol, "Tag_Symbol" },
----------------
jhenderson wrote:
> By the way: this clang-format failure might be related to your changes, so it's probably worth checking to see if it is incorrect without your changes, and if not, reformat it as part of this patch.
I didn't change the formatting in this file. I think it is not related to this patch. We could create another NFC patch to correct the formatting in ARMBuildAttrs.cpp.


================
Comment at: llvm/unittests/Support/ELFAttributeParserTest.cpp:27
+public:
+  AttributeHeaderParser(ScopedPrinter *sw)
+      : ELFAttributeParser(sw, emptyTagNameMap, "test") {}
----------------
jhenderson wrote:
> `sw` seems a bit of a random name. What does it mean?
I do not know either. I borrowed the name from ARMAttributeParser. I will change the variable name to 'printer'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023





More information about the llvm-commits mailing list