[PATCH] D67758: [llvm-lipo] Add support for archives

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap marked an inline comment as done.
alexshap added inline comments.


================
Comment at: tools/llvm-lipo/llvm-lipo.cpp:105
+// For compatibility with cctools lipo, alignment is calculated as the minimum
+// aligment of all segments. Each segments's alignment is the maximum alignment
+// from its sections
----------------
mtrent wrote:
> A segment alignment has no strong correlation to the section alignment, and the notion that the segment alignment is the maximum alignment of its sections is not true. Maybe the simplest way to say this is:
> 
> "For compatibility with cctools lipo, a file's alignment is calculated as the minimum aligment of all segments. For object files, the file's alignment is the maximum alignment of its sections."
> 
> Part of the problem is that the method here is named "calculateSegmentAlignment" but that 1) doesn't accurately describe how lipo is using that value, and 2) also doesn't completely describe what the function is doing. But cctools also has both these problems :) You have an opportunity to call this function something like "calculateSliceAlignment" or "calculateFileAlignment" which may be objectively better ... 
> 
> Be aware the segment computation is merely a guess, and it may not be exactly correct. But it's correct enough for lipo.
> 
> And be aware the behavior for object files may change in future versions of cctools.
You are right.
Thanks, will change it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67758