[PATCH] D113104: add the xcoff symbol size for the llvm-nm.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 17 01:01:37 PST 2021


jhenderson added a comment.

As noted in the other llvm-nm patch, I'll be offline from the end of today until the 4th of January, and won't be able to respond to anything in that time.



================
Comment at: llvm/test/tools/llvm-nm/XCOFF/size.test:2-4
+## Test size of label symbol
+## Test size of SD symbol
+## Test size of common symbol
----------------
Similar comment to the basic.test test case: move these comments to near the CHECK point, and add trailing full stops (sorry for not thinking about this earlier - now that this is written here, it's clear to me they belong elsewhere in the file).


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/size.test:6-7
+
+# RUN: yaml2obj --docnum=1 %s -o %t.o
+# RUN: llvm-nm  --print-size %t.o | FileCheck --check-prefix=NM-SIZE --match-full-lines %s
+
----------------
As in other test cases, don't use `--docnum` with only one input, and don't use `--check-prefix` with only one set of CHECK lines.


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/size.test:13-19
+  - Name:                    .text
+    Address:                 0x0
+    Flags:                   [ STYP_TEXT ]
+  - Name:                    .data
+    Flags:                   [ STYP_DATA ]
+  - Name:                    .bss
+    Flags:                   [ STYP_BSS ]
----------------
Remove excessive spacing. Do you need the `Address: 0x0` line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113104



More information about the llvm-commits mailing list