This is an archive of the discontinued LLVM Phabricator instance.

[PartialInlining] : Partial inlining Overhead reduction: eliminate unnecessary live-out(s)
ClosedPublic

Authored by davidxl on May 30 2017, 2:14 PM.

Details

Summary

When there are multiple outgoing edges from outlined region to the return block, a new return block is created and the original return block becomes the 'pre-return block' to factor all incoming values from the outlined region to the new return. The original return block will be part of the outlined region. The original phi in the pre-return block is pruned so that it only takes incoming values from the outlined region.

There are cases when the old phi becomes trivial (all operands have the same value) -- for instance with the same constant or same live across value (from outside the region). In such cases, leaving the phi node will make the code-extractor decide that the phi's def is a live out value defined inside the outlined region -- this leads to additional runtime overhead: stack space to store the live out value, passing the address, in addition to load/store around the outlined calls.

This patch cleans it up.

Diff Detail

Event Timeline

davidxl created this revision.May 30 2017, 2:14 PM
wmi accepted this revision.May 31 2017, 4:14 PM

LGTM. Only a minor comment.

lib/Transforms/IPO/PartialInlining.cpp
660–670

Maybe a little bit simpler to use find_if, but I am not sure.

if (find_if(PN->incoming_values(), std::bind2nd(std::not_equal_to<Value *>(), PN->getIncomingValue(0)))
        != PN->incoming_values().end())
  return (Value *)nullptr;
return PN->getIncomingValue(0);
This revision is now accepted and ready to land.May 31 2017, 4:14 PM
davide accepted this revision.May 31 2017, 4:21 PM
davide added inline comments.
lib/Transforms/IPO/PartialInlining.cpp
660–670

I don't think the find_if version is more readable, but that's just my opinion.

An alternative, which I find slightly more readable is using llvm::all_of to make sure all the operands are the same of the first, and in that case return the first and nullptr otherwise.
That would remove the need for the loop.

davidxl updated this revision to Diff 100941.May 31 2017, 4:45 PM

Use all_of idiom.

davide accepted this revision.May 31 2017, 5:03 PM

Just a comment explaining why we introduce useless PHIs (and therefore the need to remove them).
Wit that, LGTM.

This revision was automatically updated to reflect the committed changes.

thanks for your patience!