This is an archive of the discontinued LLVM Phabricator instance.

[ifs] Add --strip-needed flag
ClosedPublic

Authored by abrachet on Feb 15 2022, 5:47 PM.

Diff Detail

Event Timeline

abrachet created this revision.Feb 15 2022, 5:47 PM
abrachet requested review of this revision.Feb 15 2022, 5:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2022, 5:47 PM
abrachet updated this revision to Diff 409291.Feb 16 2022, 9:10 AM

Apply clang-format suggestion

mcgrathr accepted this revision.Feb 16 2022, 10:05 AM

LGTM but Haowei should give the final say.

This revision is now accepted and ready to land.Feb 16 2022, 10:05 AM
haowei added inline comments.Feb 16 2022, 10:09 AM
llvm/tools/llvm-ifs/llvm-ifs.cpp
106

Since this flag affects both IFS output as well as ELF output. I think it is better to rename this variable to StripNeededLibs

abrachet updated this revision to Diff 409340.Feb 16 2022, 11:14 AM
  • Changed cl::opt name
  • Clear NeededLibs vector inline instead of using separate function
abrachet marked an inline comment as done.Feb 16 2022, 11:15 AM
abrachet added inline comments.
llvm/tools/llvm-ifs/llvm-ifs.cpp
106

Done. Out of curiosity is there a reason that these other flags are only meaningful when outputting ifs? Why not have strip-undefined strip undefined syms when outputting elf stubs too?

haowei added inline comments.Feb 16 2022, 11:31 AM
llvm/tools/llvm-ifs/llvm-ifs.cpp
106

At the time when it was implemented, we thought it was unnecessary. So it only applied to IFS file. I guess it is OK to make the flag applies to the ELF as well.

haowei accepted this revision.Feb 17 2022, 11:24 AM
This revision was landed with ongoing or failed builds.Feb 17 2022, 11:25 AM
This revision was automatically updated to reflect the committed changes.
abrachet marked an inline comment as done.