Page MenuHomePhabricator

[IROutliner] Outlining branches with single entry and single exit
ClosedPublic

Authored by AndrewLitteken on Jul 28 2021, 12:58 PM.

Details

Summary

Using the similarity found from the IRSimilarity Identifier, we take regions with structural similarity, and deduplicate them into a separate function. The Code Extractor is able to provide most of this functionality.

For simplicity, we start by only outlining regions with a single entry and single exit branch, this reduces the complexity in handling phi nodes outside the region, and handling many sets of outputs for each of the different exit blocks.

Diff Detail

Event Timeline

AndrewLitteken requested review of this revision.Jul 28 2021, 12:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 28 2021, 12:58 PM

Updating tests, fixing bugs, and bringing up to date with parent patches.

paquette added inline comments.Thu, Aug 26, 4:00 PM
llvm/include/llvm/Transforms/IPO/IROutliner.h
324

just return EnableBranches?

llvm/lib/Transforms/IPO/IROutliner.cpp
36

maybe "DisableBranches" instead?

98

this sentence doesn't really make sense to me?

167

fold this if into the if above?

172

Comments?

191–202

Move this above the if and else, then you can early return from the if portion?

804

this loop could be an any_of?

833

else isn't necessary here

1461

assert that this isn't nullptr?

1515–1521

comments for this bit?

1516

can this be an any_of?

1536

fold this if into the if above?

1621

fold this if into the if above?

1780

assert that this isn't nullptr?

AndrewLitteken added inline comments.Mon, Aug 30, 2:01 PM
llvm/lib/Transforms/IPO/IROutliner.cpp
804

I don't think so? We need to look at all of the successors, and for each one not contained in, it needs to get added to the set. So we need a boolean for each block so I think this makes sense.

Updating based on comments

paquette accepted this revision.Tue, Aug 31, 3:05 PM

I think this looks fine with nits.

llvm/lib/Transforms/IPO/IROutliner.cpp
36

this could use a comment?

805

drop braces?

This revision is now accepted and ready to land.Tue, Aug 31, 3:05 PM
This revision was landed with ongoing or failed builds.Tue, Sep 7, 8:52 AM
This revision was automatically updated to reflect the committed changes.