Page MenuHomePhabricator

[IROutliner] Ensure instructions at end of candidate are excluded
Needs ReviewPublic

Authored by AndrewLitteken on Jun 11 2021, 12:00 PM.

Details

Reviewers
jroelofs
paquette
Summary

Occasionally instructions are between the last instruction in a region, and the following instruction as identified by the Candidate. This adds an extra check right before splitting a candidate that excludes the region from being split/checked for outlining to remove errors.

Tests Changed:
Tranforms/IROuutliner/outlining-extra-bitcasts.ll

Diff Detail

Event Timeline

AndrewLitteken requested review of this revision.Jun 11 2021, 12:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2021, 12:00 PM
ormris removed a subscriber: ormris.Jun 11 2021, 2:49 PM
hiraditya added inline comments.Jun 13 2021, 12:47 PM
llvm/lib/Transforms/IPO/IROutliner.cpp
1665

Do we need to split when the region is going to be ignored?

AndrewLitteken added inline comments.Jun 13 2021, 3:01 PM
llvm/lib/Transforms/IPO/IROutliner.cpp
1665

There's a chance that when the region is split, extra instructions are added changing the possibility of outlining, and it is no longer viable to be split. It might be better to have this function return a boolean if it has been split, or has not been, then continue based on that result.

Update splitCandidates to escape early if the region cannot be extracted, and checking the boolean CandidateSplit for the region rather than using IgnoreRegion,

Can you add context to the diff?

Adding context to the diff.

paquette added inline comments.Thu, Jul 1, 12:51 PM
llvm/lib/Transforms/IPO/IROutliner.cpp
132

Move EndInst above and use it here?

Also, this could use a comment.

1665

Could use a comment explaining why this wouldn't happen.

llvm/test/Transforms/IROutliner/outlining-bitcasts.ll
40

Why did these lines disappear?

53
53

Sounds like "Outlining only occurs in this case due to the lack of a cost model" should be a FIXME?

AndrewLitteken added inline comments.Fri, Jul 2, 7:27 AM
llvm/test/Transforms/IROutliner/outlining-bitcasts.ll
40

They move down to the bottom of the test, I think this was a function of the test generation script.

53

It's not really a fix me, the no cost command line argument is here to test these sorts of cases, so that it's not dependent on the way that the cost of outlining is calculated, rather how we define similarity. I can make this more clear.

Adding comments and fixes.