Page MenuHomePhabricator

[WIP][Attributor] Per function iteration and memory tracking.
Needs ReviewPublic

Authored by kuter on Aug 25 2020, 8:43 PM.

Details

Summary

This patch makes the attribute iterate on per function (and it's deps)
function basis. This way we don't have to have all AAs in memory at
once.

We do this by having a separate allocator per each function. And attributes that are not in the "Function interface" are kept there.
Function interface attributess are kept alive till the end on the fixpoint iteration.
Seeded attributes are allocated in a user provided allocator.

For doing iteration on each function separately, we track seeded attributes per function first and "bootstrap" them when it is their time.
Functions can still query what they want regardless of which function we are currently iterating in (as long as the queried AA is a interface AA).

Diff Detail

Event Timeline

kuter created this revision.Aug 25 2020, 8:43 PM
Herald added a reviewer: homerdin. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
kuter requested review of this revision.Aug 25 2020, 8:43 PM
kuter added a comment.EditedAug 25 2020, 8:46 PM

I only updated the allow_list.ll. This brakes many tests and in some cases cause invalid IR (assertion gets trigered for a tail call).

I am looking for feedback on this, there is something that I am missing.

I am looking for feedback on this, there is something that I am missing.

Please describe the approach in the commit message with sufficient detail so one does not need to reconstruct what is going on from the code.

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

Unrelated.

164

Unrelated.

185

Unrelated.

365

Documentation.

964

Documentation.

kuter added a comment.Aug 25 2020, 9:16 PM

Thank you I will update the commit message.

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

I had to make this a SmallVector

llvm/lib/Transforms/IPO/Attributor.cpp
1352–1353

This is no longer used.

kuter edited the summary of this revision. (Show Details)Aug 25 2020, 9:20 PM
kuter retitled this revision from wq[WIP][Attributor] Per function iteration and memory tracking. to [WIP][Attributor] Per function iteration and memory tracking..

Thanks for the explanation. Since the patch is not small and it is a WIP, can you add some more information about the failing tests? Maybe some specific examples?

Merge checks are helpfull, but I guess if you give few specific examples with problems, it will be easier to focus. WDYT?

kuter added a comment.EditedAug 26 2020, 2:40 AM

Thanks for the explanation. Since the patch is not small and it is a WIP, can you add some more information about the failing tests? Maybe some specific examples?

Merge checks are helpfull, but I guess if you give few specific examples with problems, it will be easier to focus. WDYT?

I wasn't sure of the issue, but I now think that there is a problem with the way things are manifested.
I think there is a problem when we manifest a attribute without manifesting it's deps and this could happen with stub attributes.

We have to create "stub" (the name should change) AAs if we query for a deleted attribute.
I used to not update the "stub" attributes but now I do, maybe we can call them "duplicate".

The reason that we end up querying deleted AAs is because we don't seed every function interface AA.
We can reduce this significantly by changing the order that we iterate on functions.

Or we can also try keeping internal function AAs "alive" until we look at it's every caller(This would also fix the problem with call base context),
Of course this would increase the memory usage. This shouldn't be too hard because of the way I coded things.

llvm/test/Transforms/Attributor/allow_list.ll
69

This could be a example to the manifest problem.

kuter updated this revision to Diff 288090.Aug 26 2020, 1:02 PM
kuter edited the summary of this revision. (Show Details)

Split unrelated changes to another patch
Handle attibutes that are created after function delete.