This is an archive of the discontinued LLVM Phabricator instance.

[Attributor] Use internalized version of non-exact functions
ClosedPublic

Authored by bbn on Jul 20 2020, 5:55 AM.

Details

Summary

This patch internalize non-exact functions and replaces of their uses
with the internalized version. Doing this enables the analysis of
non-exact functions.

We can do this because some non-exact functions with the same name
whose linkage is linkonce_odr or weak_odr should have the same
semantics, so we can safely internalize and replace use of them (the
result of the other version of this function should be the same.).
Note that not all functions can be internalized, e.g., function with
linkonce or weak linkage.

For now when specified in commandline, we internalize all functions
that meet the requirements without calculating the cost of such
internalzation.

Diff Detail

Event Timeline

bbn created this revision.Jul 20 2020, 5:55 AM
Herald added a reviewer: uenoku. · View Herald Transcript
Herald added a reviewer: homerdin. · View Herald Transcript
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
bbn updated this revision to Diff 279214.Jul 20 2020, 6:01 AM

Minor update to test

fhahn added a subscriber: fhahn.Jul 20 2020, 6:27 AM
fhahn added inline comments.
llvm/lib/Transforms/IPO/Attributor.cpp
1457

nit: we are already using the llvm namespace, there should be no need to use llvm::. same at other places in the patch,

jdoerfert added inline comments.Jul 20 2020, 6:37 AM
llvm/lib/Transforms/IPO/Attributor.cpp
97

maybe state that this is done via cloning, or at least that this is sound.

1432

Typo above.

1446

Maybe: ".internalized" ?

1457

Are there really ModuleLevelChanges?

2218

Remove the const above so you can remove the const_cast.

llvm/test/Transforms/Attributor/internalize.ll
2

Please add all 4 run lines we usually use, once with and once without the flag.
We can also use the update script then. We soon get the ability to check for new functions but for now you can use some manual lines as you did so far for those.

bbn updated this revision to Diff 280486.Jul 24 2020, 9:18 AM
  • Changes as others suggests:
    • Updated description for the command line option
    • Fixed typos
    • Changed the copied function's name from "_copied" to ".internalized"
    • Remove the const qulifier
    • Added 4 run lines of tests
    • Removed the llvm:: for the clone function
  • Other changes
    • call the CGUpdater.registerOutlinedFunction() for the internalized function
bbn marked 7 inline comments as done.Jul 24 2020, 9:19 AM
jdoerfert accepted this revision.Aug 11 2020, 10:17 PM

LGTM, minor nits below.

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

Same as below.

1446

We actually want PrivateLinkage I think. No need for llvm:: here and elsewhere.

2211

Please use GlobalValue::isInterposableLinkage here.

2222

Add a set of outer braces, this makes it easier to read.

This revision is now accepted and ready to land.Aug 11 2020, 10:17 PM
bbn updated this revision to Diff 285550.Aug 13 2020, 8:34 PM
This revision was automatically updated to reflect the committed changes.