Page MenuHomePhabricator

[Attributor][WIP] Cost interface for function internalization
Needs ReviewPublic

Authored by bbn on Sun, Jul 26, 8:48 PM.

Details

Summary

Introduce a cost interface which determines whether internalizing a function is worthwhile.

Diff Detail

Event Timeline

bbn created this revision.Sun, Jul 26, 8:48 PM
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 added a comment.Sun, Jul 26, 8:49 PM

For now the patch only contains the interface and is still WIP. I will update it with more detailed algorithm later.

jdoerfert added inline comments.Mon, Jul 27, 5:46 AM
llvm/include/llvm/Transforms/IPO/Attributor.h
145

Make the names more descriptive and add plenty doxygen documentation for all of them. Maybe put them all in a struct so the user could modify the values by changing the struct members.

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

I somehow think the method called here should be "shouldInternalize" or something. The problem here is that it "looks" like we internalize if there is an associated cost, e.g., the cost is not zero.

bbn updated this revision to Diff 280905.Mon, Jul 27, 7:18 AM
  • Cache the InternalizeCost for every function

What I am thinking is that we need to calculate the cost after all dependencies between
AAs become clear. So I think we need to do the cost analysis after the fix point
iteration (or after the first update is done).

bbn marked an inline comment as done.Mon, Jul 27, 7:22 AM
bbn added inline comments.
llvm/include/llvm/Transforms/IPO/Attributor.h
145

Sure, I will add docs once I figure out the when to do such cost analysis and what
can increase the cost of internalization (for now I can only think of the number of
instructions) and what can be the "bonus".

bbn updated this revision to Diff 280910.Mon, Jul 27, 7:26 AM

Changed some function names.

jdoerfert added inline comments.Mon, Aug 10, 7:15 PM
llvm/include/llvm/Transforms/IPO/Attributor.h
195

There is no need for dynamic allocation here. Let's just return a value, it's only 16 bytes on most systems so easy to copy and with the advantage that we don't leak memory ;)