[PATCH] D118193: [llvm-nm] add a new option -X to specify the type of object file llvm-nm should examine

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-nm.rst:133
+   line or from big archive files and llvm-nm should examine. The mode must be one of the following:
+   32
+         Processes only 32-bit object files.
----------------
MaskRay wrote:
> Can you run `ninja docs-llvm-html` and inspect whether the .rst renders properly?
> 
> Ensure sphinx-build exists. Configure your build with `-DLLVM_ENABLE_SPHINX=ON`.
thanks


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/option-X.test:12
+# BIT32:     00000000 D var32
+# BIT32-NOT: 0000000000000000 D var64
+
----------------
MaskRay wrote:
> If you use `--implicit-check-not={{.}}`, the `NOT` pattern should be unneeded.
the BIT32-NOT is for 
RUN: llvm-nm -X32 %t.a | FileCheck --check-prefixes=BIT32 %s 


================
Comment at: llvm/test/tools/llvm-nm/option-X-elf-bitcode.test:6
+
+# RUN: clang -emit-llvm -m32 -o %t32.bc -c %t32.c
+# RUN: clang -emit-llvm -m64 -o %t64.bc -c %t64.c
----------------
MaskRay wrote:
> Because of project layering, llvm tests cannot use clang. You need to use `llc` to generate the test.
> 
> Consider `split-file`. There are quite few tests in `llvm/test/tools/`
thanks


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:87
 enum OutputFormatTy { bsd, sysv, posix, darwin, just_symbols };
+enum class BitModeTy { Bit32, Bit64, Bit32_64, Any };
 } // namespace
----------------
MaskRay wrote:
> If Any is not that different from Bit32_64, just remove Any.
yes, we have maybe has  to support "the old pre-AIX v5 XCOFF64." later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118193