Page MenuHomePhabricator

[Attributor] Add a phase flag to Attributor
ClosedPublic

Authored by okura on Aug 26 2020, 10:27 PM.

Details

Summary

Add a new flag that indicates which stage in the process we are in.
This flag is introduced for handling behavior of getAAFor according to the stage. (discussed in D86635)

Diff Detail

Event Timeline

okura created this revision.Aug 26 2020, 10:27 PM
Herald added a reviewer: homerdin. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
okura requested review of this revision.Aug 26 2020, 10:27 PM
kuter added a comment.EditedAug 26 2020, 11:56 PM

Thank you for creating this. I was thinking of something like this rather than having multiple flags.
In D86587 I iterate on functions multiple times and I delete AAs early after manifesting them.
(Manifest happens multiple times).
I think most of the state change should happen in the ::run function.

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

The state is stuck as MANIFEST

Patch is missing description.

okura edited the summary of this revision. (Show Details)Aug 27 2020, 12:37 AM

Patch is missing description.

added description, thanks

Thank you for creating this. I was thinking of something like this rather than having multiple flags.
In D86587 I iterate on functions multiple times and I delete AAs early after manifesting them.
(Manifest happens multiple times).
I think most of the state change should happen in the ::run function.

OK, I'll move state changes into ::run

Thank you for creating this. I was thinking of something like this rather than having multiple flags.
In D86587 I iterate on functions multiple times and I delete AAs early after manifesting them.
(Manifest happens multiple times).
I think most of the state change should happen in the ::run function.

OK, I'll move state changes into ::run

Thank you.

okura updated this revision to Diff 288225.Aug 27 2020, 1:15 AM
  • use scoped enum
  • move status change into Attributor::run
This revision is now accepted and ready to land.Aug 27 2020, 7:46 AM
This revision was automatically updated to reflect the committed changes.