[PATCH] D142479: [AIX] support 64bit global symbol table for big archive

[Digger Lin via Phabricator via llvm-commits]

DiggerLin marked 3 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/include/llvm/Object/Archive.h:408
   uint64_t LastChildOffset = 0;
+  SmallString<0> GlobalSymbolTableBuf;
 
----------------
jhenderson wrote:
> I think it's unlikely that this buffer is ever going to be "small", so it seems likely that it would just be better to store this a `std::string` ultimately.
there are null characters between each symbol name to separate the symbol names in the  global symbol table, I do not think we can use the std::string to store it.

and the base class of SmallString is SmallVector, According to the comment in the SmallVector.h, "SmallVectors are used for buffering bitcode output - which can exceed 4GB.", 


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:423
+
+  // The is64BitSymbolicFile() in the llvm-ar.cpp will be run before this
+  // function. If there is unsupported file format, it will be fail at that
----------------
jhenderson wrote:
> The fact that there's another `is64BitSymbolFile()` should tell you that you need to reorganise some code to avoid code duplication.
I createed a new NFC patch (https://reviews.llvm.org/D143097)  which will add a new virtual API Is64bit() for SymbolicFile 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142479