[PATCH] D132494: [AIX] llvm-nm support environment "OBJECT_MODE" for option -X on AIX OS

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-nm.rst:144-150
+   In AIX OS, the default is to process 32-bit object files (ignore 64-bit objects). The mode can also
+  be set with the OBJECT_MODE environment variable. For example, OBJECT_MODE=64 causes ar to
+  process any 64-bit objects and ignore 32-bit objects. The -X flag overrides the OBJECT_MODE
+  variable.
+
+   In no AIX OS, default is to process all the supported object files. And do not support the
+  OBJECT_MODE environment variable.
----------------
jhenderson wrote:
> Some suggested rewording.
> 
> Please reflow all lines to an 80 character width.
thanks


================
Comment at: llvm/test/tools/llvm-nm/option-X-AIX.test:42-43
+
+# RUN: yaml2obj -DFLAG=0x1DF %s -o %t_xcoff32.o
+# RUN: yaml2obj -DFLAG=0x1F7 %s -o %t_xcoff64.o
+# RUN: rm -rf %t.a
----------------
jhenderson wrote:
> Is there a reason you're using bitcode files for the first set of tests and XCOFF files here? It seems like you could just use one set of files for all the testing.
there is no reason to using bitcode files for the first set of tests.  I chose the bit code and XCOFF just because the format are used in AIX OS.  other object file format are not used in AIX OS. 


================
Comment at: llvm/test/tools/llvm-nm/option-X-Non-AIX.test:24-25
+
+# RUN: llvm-nm --format=just-symbols %t_elf32.o %t_elf64.o | \
+# RUN:   FileCheck %s -DFILE32=%t_elf32.o -DFILE64=%t_elf64.o --check-prefixes=ELF32,ELF64
+
----------------
jhenderson wrote:
> There's no need to have the ELF, bitcode, XCOFF and archive cases in separate llvm-nm invocations - they can all be one llvm-nm call.
I combine  ELF, bitcode, XCOFF together , and put archive  in separate test, otherwise the test is too complication. 


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:2380
+  if (HostTriple.isOSAIX()) {
+    auto getBitMode = [](const char *RawBitMode) {
+      return StringSwitch<BitModeTy>(RawBitMode)
----------------
jhenderson wrote:
> The lambda doesn't add anything, since it's only called once.
thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132494