[PATCH] D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 2 00:49:12 PST 2023
jhenderson added a comment.
No need to move the tests unless you want to. Your logic for where they are is as good as anything else.
================
Comment at: llvm/test/tools/llvm-dwarfdump/AMDGPU/amdgcn-relocs.yaml:1
+## Tests llvm-dwarfdump handling of AMDGPU relocations. We provide a .debug_info
+## section with multiple DW_AT_high_pc entries (that's one of the attributes for
----------------
Does this test need renaming, since it covers R600 too?
================
Comment at: llvm/test/tools/llvm-dwarfdump/AMDGPU/amdgcn-relocs.yaml:17
+
+
+# AMDGCN-UNKNOWN: -: Error in creating MCRegInfo
----------------
Nit: I'd get rid of the extra blank line (but I would have one before the start of the YAML).
================
Comment at: llvm/test/tools/llvm-dwarfdump/AMDGPU/amdgcn-relocs.yaml:18-19
+
+# AMDGCN-UNKNOWN: -: Error in creating MCRegInfo
+# AMDGCN-KNOWN-NOT: -: Error in creating MCRegInfo
+# AMDGCN: -: file format elf64-amdgpu
----------------
Up to you, but you could change these to be shared with the R600 cases too, since the message is the same.
That being said, why are you checking them?
================
Comment at: llvm/test/tools/llvm-dwarfdump/AMDGPU/amdgcn-relocs.yaml:138-141
+
+# R600-UNKNOWN: -: Error in creating MCRegInfo
+# R600-KNOWN-NOT: -: Error in creating MCRegInfo
+# R600: -: file format elf32-amdgpu
----------------
Again, I'd get rid of the extra blank line, and then add one after the CHECK, but I don't feel strongly about it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144301/new/
https://reviews.llvm.org/D144301
More information about the llvm-commits
mailing list