This is an archive of the discontinued LLVM Phabricator instance.

[ifs] Add option to hide undefined symbols
ClosedPublic

Authored by haowei on Aug 19 2021, 5:43 PM.

Details

Summary

This change add an option to llvm-ifs to hide undefined symbols from its output.

Diff Detail

Event Timeline

haowei created this revision.Aug 19 2021, 5:43 PM
haowei requested review of this revision.Aug 19 2021, 5:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 19 2021, 5:43 PM

Would it be possible to include a test for this option?

llvm/lib/InterfaceStub/IFSHandler.cpp
333–334

I'd use a for loop here.

337

You need to update it since the iterator will be invalidated by the erase operation.

llvm/tools/llvm-ifs/llvm-ifs.cpp
451–453

No need for braces.

mcgrathr added inline comments.Aug 23 2021, 1:38 PM
llvm/lib/InterfaceStub/IFSHandler.cpp
331

It's not clear why this function takes the various arguments since it has only one call site that uses fixed values.

337

it = erase(it); should work.

haowei updated this revision to Diff 368450.Aug 24 2021, 1:29 PM
haowei marked 3 inline comments as done.Aug 24 2021, 1:50 PM

I forgot to attach the test into git in the previous commit. They were attached int this change.

llvm/lib/InterfaceStub/IFSHandler.cpp
331

I was plan to make it generic so next time if we need to strip any more symbols based on a different metric, I don't need to write a new function. But I think it does not really matter since it is quite simple.

haowei updated this revision to Diff 368461.Aug 24 2021, 2:07 PM
haowei marked an inline comment as done.

Rebase to latest HEAD

haowei updated this revision to Diff 368463.Aug 24 2021, 2:10 PM
haowei marked an inline comment as done.
phosek added inline comments.Aug 24 2021, 3:11 PM
llvm/tools/llvm-ifs/llvm-ifs.cpp
103

Small suggestion, I'd call this just strip-undefined, the symbol part is already implied and it better matches spelling used by tools like llvm-objcopy.

haowei updated this revision to Diff 368677.Aug 25 2021, 10:24 AM
haowei marked an inline comment as done.
phosek accepted this revision.Aug 25 2021, 10:29 AM

LGTM

This revision is now accepted and ready to land.Aug 25 2021, 10:29 AM
This revision was automatically updated to reflect the committed changes.