[PATCH] D65240: [XCOFF][AIX] Generate symbol table entries with llvm-readobj

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:478
+  if (SymbolEntPtr >
+      reinterpret_cast<uintptr_t>(reinterpret_cast<const char *>(SymbolTblPtr) +
+                                  SYM_TAB_ENTRY_SIZE * NumberOfSymTableEntries))
----------------
`getWithOffset`?


================
Comment at: llvm/test/tools/llvm-readobj/xcoff-symbols.test:1
+# RUN: llvm-readobj --symbols %p/Inputs/aix_xcoff_xlc_test8.o | \
+# RUN: FileCheck --check-prefix=SYMBOL32 %s
----------------
jhenderson wrote:
> A few test comments from me:
> 
> Add a brief comment to the top of the file explaining the purpose of the test.
> 
> You don't need to prepend the `RUN:` lines with a '#' character.
> 
> Add a single blank line between the end of the RUN: lines and the start of the CHECK patterns.
> 
> I take it yaml2obj doesn't have XCOFF support? Assuming that's the case, is it possible to use something like llvm-mc to build this at test time rather than use the pre-canned binary?  In general we've been trying to avoid adding pre-canned binaries to the llvm-readobj tests, as they are opaque and hard to maintain.
> I take it yaml2obj doesn't have XCOFF support? Assuming that's the case, is it possible to use something like llvm-mc to build this at test time rather than use the pre-canned binary? In general we've been trying to avoid adding pre-canned binaries to the llvm-readobj tests, as they are opaque and hard to maintain.

We are using the pre-canned binaries for testing since we need the tools support first, to be able to write lit tests as we add backend XCOFF functionality. Once we have enough functionality in the XCOFFObjectWriter we can remove the pre-canned binaries and generate them with llvm-mc as you suggest.


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

https://reviews.llvm.org/D65240