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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 12 01:47:58 PST 2020


jhenderson added a comment.

(I don't know anything about RISC-V - my comments are purely from an llvm-readobj developer's point of view).



================
Comment at: llvm/test/tools/llvm-readobj/ELF/riscv-attribute.s:1
+# RUN: llvm-mc -triple riscv32 -filetype asm -o - %s | FileCheck %s
+# RUN: llvm-mc -triple riscv32 -filetype obj -o - %s \
----------------
This is going to need a REQUIRES, right?

Also, we prefer to have a top-level comment in new llvm-readobj tests that explains the point of the test.

I also wonder whether it would be less circular in the testing if you added support in yaml2obj to generate such a section, and then use that to write the llvm-readobj test. I'm not sure either way though.


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