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

[Jason Liu via Phabricator via llvm-commits]

jasonliu marked 19 inline comments as done.
jasonliu added inline comments.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:122
+  support::ubig32_t Stab;
+  support::ubig16_t Snstab;
+};
----------------
hubert.reinterpretcast wrote:
> `StabSecNum`, like `TypeChkSectNum`.
Changed it to StabSectNum.


================
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
----------------
sfertile wrote:
> 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.
Thanks for the comments. Addressed them in the new revision.

Yes, we haven't implement XCOFF support for yaml2obj yet. And llvm-mc do not yet have enough functionality to generate all the symbol entries kind for our testing, although we are working towards that goal. And we are actually going to use llvm-readobj to test the correctness of our llvm-mc obj gen. 
So there is a chicken-and-egg problem here for us. And what we are choosing to do is to use a pre-canned binary that we know is correct (generated from xlc) to test the correctness of the tooling (llvm-readobj). Then later use this tooling to test the object file generated by llc/llvm-mc. 


================
Comment at: llvm/test/tools/llvm-readobj/xcoff-symbols.test:21
+SYMBOL32-NEXT:       Type: XFT_FN (0x0)
+SYMBOL32-NEXT:       AuxType: 0x0
+SYMBOL32-NEXT:     }
----------------
hubert.reinterpretcast wrote:
> There is no such field defined for XCOFF32.
Removed this for printing XCOFF32.


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

https://reviews.llvm.org/D65240