[PATCH] D144301: [dwarfdump][AMDGPU] Support EF_AMDGPU_MACH_NONE

[James Henderson via Phabricator via llvm-commits]

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