This is an archive of the discontinued LLVM Phabricator instance.

[StructurizeCFG] Use UniformityAnalysis instead of DivergenceAnalysis
ClosedPublic

Authored by Pierre-vh on Mar 9 2023, 6:02 AM.

Diff Detail

Event Timeline

Pierre-vh created this revision.Mar 9 2023, 6:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 6:02 AM
Pierre-vh requested review of this revision.Mar 9 2023, 6:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 6:02 AM
foad added a subscriber: nhaehnle.Mar 9 2023, 6:06 AM
foad added inline comments.
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
802

Does UA need something like removeValue? I see it was added by @nhaehnle in D43743 "to avoid dangling pointers as a matter of defensive programming".

Pierre-vh added inline comments.Mar 9 2023, 6:09 AM
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
802

I'm not too familiar with UA's implementation so I can't answer this, @sameerds does UA need anything like this?

sameerds added inline comments.Mar 9 2023, 11:32 PM
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
802

Yeah, given the defensive nature of the comment, I thought we should put this in UA. But turns out that we don't really need to, because UA does not store pointers to terminator instructions. The side-effect of the structurizer is to delete and recreate terminators, but in the UA, a query on a terminator is actually done by looking up its parent basic block.

Also noteable is that only LegacyDA implemented this "remove" API. Although legacy DA acts as a wrapper to the newer DA, this particular function was never implemented in the latter.

In short, I think we don't need to implement this in UA.

sameerds accepted this revision.Mar 9 2023, 11:34 PM

Quite possibly the scariest test of UA so far! I am definitely ducking behind a vibranium shield!

This revision is now accepted and ready to land.Mar 9 2023, 11:34 PM
foad accepted this revision.Mar 10 2023, 2:06 AM
foad added inline comments.
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
802

Sounds good, thanks for explaining!

This revision was landed with ongoing or failed builds.Mar 13 2023, 12:31 AM
This revision was automatically updated to reflect the committed changes.