This is an archive of the discontinued LLVM Phabricator instance.

[Attributor] Conditinoally delete fns
ClosedPublic

Authored by wsmoses on Feb 22 2021, 4:13 PM.

Details

Summary

Allow the attributor to delete functions only if requested

Diff Detail

Event Timeline

wsmoses created this revision.Feb 22 2021, 4:13 PM
wsmoses requested review of this revision.Feb 22 2021, 4:13 PM
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript

Attributor::deleteAfterManifest also needs to be updated or removed (together with the use).

llvm/include/llvm/Transforms/IPO/Attributor.h
1665

Nit: .

llvm/lib/Transforms/IPO/Attributor.cpp
1221

Early exit out of this function if we don't delete, all what is happening here is not necessary then.

sstefan1 added inline comments.Feb 23 2021, 1:26 AM
llvm/test/Transforms/Attributor/nodelete.ll
2

Maybe generate this with update_test_checks.py? I guess it would be easier to update and also for consistency.

jdoerfert added inline comments.Feb 23 2021, 7:51 AM
llvm/test/Transforms/Attributor/nodelete.ll
2

I missed this. Use the same run lines as other tests and the update test script please.

wsmoses updated this revision to Diff 325862.Feb 23 2021, 11:49 AM

WIP: update code per comments, still needs test fixes

wsmoses updated this revision to Diff 326873.Feb 26 2021, 7:36 PM
wsmoses marked 2 inline comments as done.

Update tests

wsmoses updated this revision to Diff 326874.Feb 26 2021, 7:48 PM

Recreate test using script

wsmoses marked 2 inline comments as done.Feb 26 2021, 7:49 PM
wsmoses added inline comments.
llvm/test/Transforms/Attributor/nodelete.ll
2

Using the same line as others reveals a similar issue for non cgss mode. Is it preferable to leave those tests disabled for the moment (like below) or to also disable function removal for that mode?

sstefan1 added inline comments.Feb 27 2021, 10:11 AM
llvm/test/Transforms/Attributor/nodelete.ll
2

I guess we should do that while we are at it.

wsmoses updated this revision to Diff 326916.Feb 27 2021, 11:14 AM
wsmoses marked an inline comment as done.

Fix attributor nodelete test

This revision is now accepted and ready to land.Feb 27 2021, 2:15 PM
This revision was automatically updated to reflect the committed changes.
wsmoses marked an inline comment as done.