[PATCH] D71602: [llvm-readobj][llvm-objdump][test] - Cleanup testing of dynamic tags dumping.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/elf-dynamic-section-machine-specific.test:1-3
+## Here we test how machine-specific dynamic tags are dumped.
+
 ## Test that hexagon machine-specific tags can be dumped.
----------------
I'd suggest some wording changes:

1) Top-level comment should be something like "In this test we test..."
2) Individual case comments could be simplified to "Case 1: Hexagon" or "Hexagon tags"

And I wouldn't put a blank line after the comments at the start of each machine case, because the comment is tied to the respective RUN lines (just like you wouldn't put a comment for an if statement in C++ with a blank line between it and the if). 


================
Comment at: llvm/test/tools/llvm-objdump/elf-dynamic-section-machine-specific.test:44-45
 
+## FIXME: llvm-objdump does not print spaces after MIPS_DELTA_INSTANCE_NO, MIPS_PROTECTED_GOTIDX
+##        and other long tags. The output looks broken because of that.
+
----------------
I'd move this comment to the first individual line that this applies to. Are you planning on fixing this soon? If not, I'd file a bug and include a link to that bug with this comment.


================
Comment at: llvm/test/tools/llvm-objdump/elf-dynamic-section-machine-specific.test:66
-# MIPS-NEXT:   MIPS_RLD_MAP_REL     0x0000000000001000
-# MIPS-NEXT:   <unknown:>0x1234abcd 0x0000000000000001
 
----------------
You've removed testing for these unknown values, but is there any testing for them anywhere else?


================
Comment at: llvm/test/tools/llvm-objdump/elf-dynamic-section-machine-specific.test:287
+      - Tag:   DT_AARCH64_BTI_PLT
+        Value: 0
+      - Tag:   DT_AARCH64_PAC_PLT
----------------
Let's assign these some (arbitrary) values.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/dynamic-tags-machine-specific.test:1
-# Test that hexagon machine-specific tags can be dumped.
-# RUN: yaml2obj --docnum=1 %S/Inputs/elf-dynamic-tags-machine-specific.yaml -o %t.hex
+## Here we test how machine-specific dynamic tags are dumped.
+
----------------
Basically all my coments for the objdump test also apply here too.


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

https://reviews.llvm.org/D71602