[PATCH] D143537: [Object][NFC] Factor out computeHeadersSize.

Jacek Caban via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 9 07:09:53 PST 2023


jacek added a comment.

In D143537#4114220 <https://reviews.llvm.org/D143537#4114220>, @MaskRay wrote:

> The `computeSymbolTableSize` change is reasonable. Other changes increase the number of lines. I do not look at follow-ups so cannot tell whether this is reasonable.

I updated the diff with suggested change. I need separated HeaderSize in later patches to avoid calling computeSymbolTableHeaderSize multiple times, but I moved adding variables later in the series.

Other changes are mostly meant to pass objects' offsets to writeSymbolTable instead of computing it there from scratch. That computation is trivial now, but it becomes more tricky with later patches, as it would need to take second symbol table, string table and EC symbols table into account (both in writeSymbolTable and in new functions writing additional tables).



================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:416
+  Size += HeaderSize + SymtabSize;
+  return Size;
+}
----------------
MaskRay wrote:
> `return strlen("!<arch>\n") + computeSymbolTableHeaderSize() + SymtabSize;`
> 
> The compiler will optimize out `strlen`. Avoid trivially-used-once variables.
I may change it here, but then I'd need to introduce them in later patches. HeaderSize is useful, because it avoids calling computeSymbolTableHeaderSize up to 3 times, when we have another headers for the second symbol table and EC symbols. 


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

https://reviews.llvm.org/D143537



More information about the llvm-commits mailing list