DiggerLin added inline comments.
================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:237
static bool Verbose = false; ///< 'v' modifier
-static bool Symtab = true; ///< 's' modifier
+static WriteSymTabType Symtab = true; ///< 's' modifier
static bool Deterministic = true; ///< 'D' and 'U' modifiers
----------------
jhenderson wrote:
> Maybe I'm missing something, but I don't see why you need to make this a
> custom type. You already have the BitMode value that you read in from the
> environment/command-line, and so you can just use that in conjunction with
> the `Symtab` boolean to determine what symbol table to write, surely?
the Symtab are use to specify whether the symbol table need to write(for
non-AIX). and what the symbol table need to write for AIX OS.
there are function like
```
writeArchiveToBuffer(ArrayRef<NewArchiveMember> NewMembers,
WriteSymTabType WriteSymtab, object::Archive::Kind Kind,
bool Deterministic, bool Thin)
```
and
```
Error writeArchive(StringRef ArcName, ArrayRef<NewArchiveMember> NewMembers,
WriteSymTabType WriteSymtab, object::Archive::Kind Kind,
bool Deterministic, bool Thin,
std::unique_ptr<MemoryBuffer> OldArchiveBuf = nullptr);
```
call the function as
```
writeArchiveToBuffer(M.second.getMembers(),
/*WriteSymtab=*/true,
/*Kind=*/object::Archive::K_DARWIN,
C.Deterministic,
/*Thin=*/false);
```
I do not want to change the calling parameter of the /*WriteSymtab=*/true,
I introduced a new class WriteSymTabType which has following API
```
WriteSymTabType(bool PrintSym) { Value = PrintSym ? Both : None; }
void operator=(bool PrintSym) { Value = PrintSym ? Both : None; }
operator bool() { return Value != None; }
```
================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1090-1091
+
+ // If BigArchive, if there is 32-bit globol symbol table, we still use -X64
+ // to generate the 64-bit global symbol table. Same as -X32 option.
+ if (OldArchive->kind() == object::Archive::K_AIXBIG) {
----------------
jhenderson wrote:
> Just to make sure I'm following what this is trying to say: for Big Archives,
> a -X64 specification will ignore the 32-bit symbol table and generate a
> 64-bit one (the 32-bit one will remain in place, if present), and using -X32
> will cause a 32-bit symbol table to be generated, right?
>
> Assuming that's the case, I suggest the following as the comment:
>
> "For archives in the Big Archive format, the bit mode option specifies which
> symbol table to generate. The presence of a symbol table that does not match
> the specified bit mode does not prevent creation of the symbol table that has
> been requested."
yes, you are correct. I change the comment to your suggest.
================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1421
+ // specified.
+ if (object::Archive::getDefaultKindForHost() == object::Archive::K_AIXBIG) {
+ BitMode = getBitMode(getenv("OBJECT_MODE"));
----------------
jhenderson wrote:
> I think you can share this code with the llvm-ar version of the code (put it
> in a function). You can either add an "AcceptAny" member to that function's
> argument, or simply check if the value is "Any" after the function here.
In AIX OS ,since ranlib do not support -Xany. it is different to share code
with llvm-ar
-bash-5.0$ ranlib -Xany
0654-602 The specified object mode is not valid.
Specify -X32, -X64, or -X32_64.
================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1423
+ BitMode = getBitMode(getenv("OBJECT_MODE"));
+ // -X option in ranlib do not accept "any"
+ if (BitMode == BitModeTy::Unknown || BitMode == BitModeTy::Any)
----------------
jhenderson wrote:
> Does it treat "any" as "32" or does it report an error? If the latter, should
> we not do the same here?
-Xany , I will report an error.
================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1446
+ } else if (arg.front() == 'X') {
+ if (object::Archive::getDefaultKindForHost() ==
+ object::Archive::K_AIXBIG) {
----------------
jhenderson wrote:
> Similar to above, could we refactor things slightly to share the code for
> parsing the -X option between llvm-ranlib and llvm-ar?
the logic of parsing option is different with ar_main, it is difficult to share
the code. and the llvm-ranlib do not support -Xany, and there only a few lines
of duplication code, I do not think it worth us to put effort to share the code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142660/new/
https://reviews.llvm.org/D142660
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
