[PATCH] D60502: [llvm-nm] Add --special-syms

Tue Apr 30 01:41:46 PDT 2019

jhenderson added inline comments.

================
Comment at: test/tools/llvm-nm/AArch64/special-syms.test:2
+# RUN: yaml2obj %s > %t
+# Test --special-syms flag. Currently this flag is no-op, so outputs with and w/o
+# this flag should be identical. GNU nm doesn't show ARM and AArch64 special symbols
----------------
is no-op -> is a no-op

We should probably avoid abbreviations like w/o since they aren't necessary and may be hard to read for some people.

================
Comment at: test/tools/llvm-nm/AArch64/special-syms.test:8
+# RUN: llvm-nm %t | FileCheck %s
+# RUN: llvm-nm %t --special-syms | FileCheck %s
+
----------------
Can you confirm that if you run GNU nm on %t output that it produces/does not produce the symbol as appropriate? I just want to make sure that the symbols have been correctly setup to flag up any behaviour change if there is any.

================
Comment at: test/tools/llvm-nm/AArch64/special-syms.test:21
+    Address:         0x1000
+    AddressAlign:    0x0000000000000010
+    Size:            64
----------------
You can make this test slightly more concise by getting rid of the AddressAlign fields at least, and probably Size too.

================
Comment at: test/tools/llvm-nm/AArch64/special-syms.test:33-34
+    Section:  .text
+    Value:    0
+    Size:     0
+  - Name:     $d.1
----------------
You can delete Value and Size here and below. I don't think you need to explicitly state the Type either.

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

https://reviews.llvm.org/D60502