Page MenuHomePhabricator

[IRSim][IROutliner] Adding the extraction basics for the IROutliner.
Needs ReviewPublic

Authored by AndrewLitteken on Sep 1 2020, 1:23 PM.

Details

Reviewers
paquette
Summary

Extracting the similar regions is the first step in the IROutliner.

Using the IRSimilarityIdentifier, we collect the SimilarityGroups and sort them by how many instructions will be removed. Each IRSimilarityCandidate is used to define an OutlinableRegion. Each region is ordered by their occurrence in the Module and the regions that are not compatible with previously outlined regions are discarded.

Each region is then extracted with the CodeExtractor into its own function.
We test that correctly extract in:

  • test/Transforms/IROutliner/extraction.ll
  • test/Transforms/IROutliner/address-taken.ll
  • test/Transforms/IROutliner/outlining-same-globals.ll
  • test/Transforms/IROutliner/outlining-same-constants.ll
  • test/Transforms/IROutliner/outlining-different-structure.ll

Diff Detail

Event Timeline

AndrewLitteken created this revision.Sep 1 2020, 1:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 1 2020, 1:23 PM
AndrewLitteken requested review of this revision.Sep 1 2020, 1:23 PM
AndrewLitteken set the repository for this revision to rG LLVM Github Monorepo.
AndrewLitteken added a reviewer: paquette.

Updating for clang-format.

Hi Andrew,

This stuff looks really interesting! I've seen a whole bunch of review links on the list, but I'm particularly interested in seeing how you're doing your cost modelling, so where can I find that?

Hi Andrew,

This stuff looks really interesting! I've seen a whole bunch of review links on the list, but I'm particularly interested in seeing how you're doing your cost modelling, so where can I find that?

I just didn't have the cost modeling up yet, you can find it here: https://reviews.llvm.org/D87299

jroelofs added inline comments.
llvm/include/llvm/Transforms/IPO/IROutliner.h
175
llvm/lib/Transforms/IPO/IROutliner.cpp
239

Is it guaranteed that we'll find precisely one CallInst in this search? Should there be assertions to that effect?

Also, is it worth adding a break in the inner if to early exit once it has been found?

278

Can sink this to where the .clear() call is, since it's not used until then.

304

should make the copy cheaper

llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
863

Out of curiosity, how did you decide that this was the optimal spot in the pass stack for it?

AndrewLitteken added inline comments.Sep 15 2020, 4:11 PM
llvm/lib/Transforms/IPO/IROutliner.cpp
239

There may be more than one CallInst due to lifetime instructions added by the code extractor, but there will not be more than one call to the outlined function. So a break would be appropriate but an assertion would not be.

jroelofs added inline comments.Sep 15 2020, 4:12 PM
llvm/lib/Transforms/IPO/IROutliner.cpp
239

👍

Vector use cleanup.

Different handling of getSimilarity() to handle optionals.

Fixing some small nits.

Removing dead variable.