This is an archive of the discontinued LLVM Phabricator instance.

[llvm-lipo] Implement alignment function in -create
ClosedPublic

Authored by anushabasana on Jul 17 2019, 10:21 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

anushabasana created this revision.Jul 17 2019, 10:21 AM
anushabasana changed the visibility from "Public (No Login Required)" to "anushabasana (Anusha Basana)".
anushabasana changed the edit policy from "All Users" to "anushabasana (Anusha Basana)".
anushabasana removed a project: Restricted Project.
anushabasana removed subscribers: hiraditya, llvm-commits.
Herald added a project: Restricted Project. · View Herald TranscriptJul 17 2019, 10:32 AM
anushabasana changed the visibility from "anushabasana (Anusha Basana)" to "Public (No Login Required)".
anushabasana changed the edit policy from "anushabasana (Anusha Basana)" to "All Users".
anushabasana updated this revision to Diff 210371.EditedJul 17 2019, 11:07 AM

Include changes: spacing and compile error fix

compnerd added inline comments.Jul 20 2019, 12:10 PM
llvm/lib/Object/MachOUniversal.cpp
164 ↗(On Diff #210371)

Personally, I would have split the changes in this file and in MachOUniversal.h into a separate change. That change is a very simple change that could probably merged immediately.

llvm/test/tools/llvm-lipo/Inputs/CPU10-slice.yaml
1 ↗(On Diff #210371)

CPU10 is not particularly helpful. Would be nice to use the human readable form (m88k).

llvm/test/tools/llvm-lipo/Inputs/CPU14-slice.yaml
1 ↗(On Diff #210371)

Similar for the file name

llvm/tools/llvm-lipo/llvm-lipo.cpp
350 ↗(On Diff #210371)

ITYM segment instead of all load commands. The alignment of something like LC_REEXPORT_DYLIB seems irrelevant to this or am I mistaken on that?

llvm/tools/llvm-lipo/llvm-lipo.cpp
352 ↗(On Diff #210371)

const MachOObjectFile *ObjectFile -> const MachOObjectFile &O

anushabasana marked 3 inline comments as done.Jul 22 2019, 2:31 PM
anushabasana added inline comments.
llvm/test/tools/llvm-lipo/Inputs/CPU10-slice.yaml
1 ↗(On Diff #210371)

This file and CPU14 are both intended to go into calculateSegmentAlignment() since they are not any of the valid architectures. I just manually changes the CPU types for testing. What would the best naming convention be for this situation?

Address review comment + removed public MaxSectionAlignment variable to different diff.

anushabasana marked an inline comment as done.Jul 22 2019, 2:40 PM
compnerd accepted this revision.Jul 23 2019, 9:16 PM
This revision is now accepted and ready to land.Jul 23 2019, 9:16 PM
llvm/tools/llvm-lipo/llvm-lipo.cpp
355 ↗(On Diff #211198)

const bool

anushabasana marked 3 inline comments as done.Jul 24 2019, 3:47 PM
This revision was automatically updated to reflect the committed changes.
llvm/trunk/test/tools/llvm-lipo/create-default-alignment.test