This is an archive of the discontinued LLVM Phabricator instance.

PredicateInfo: Handle critical edges
ClosedPublic

Authored by dberlin on Feb 6 2017, 1:47 PM.

Details

Summary

This adds support for placing predicateinfo such that it affects critical edges.

This fixes the issues mentioned by Nuno on the mailing list.

Depends on D29519

Diff Detail

Repository
rL LLVM

Event Timeline

dberlin created this revision.Feb 6 2017, 1:47 PM
dberlin updated this revision to Diff 87514.Feb 7 2017, 1:40 PM
  • Clean up code.
dberlin updated this revision to Diff 87515.Feb 7 2017, 1:42 PM
  • Clean up code.
  • Add more testcases
davide accepted this revision.Feb 12 2017, 12:28 PM

LGTM (modulo a comment). Nuno, What do you think?

lib/Transforms/Utils/PredicateInfo.cpp
124–128 ↗(On Diff #88089)

So, in the case ABlockEdge == BBBlockEdge` you fall down and use Def/U to determine the ordering.
Unless I'm missing something, can't this be folded inside the std::tie() ? (sorry if I'm terribly mistaken here)

This revision is now accepted and ready to land.Feb 12 2017, 12:28 PM

Just to note, on the thread about this, Nuno said:
"I just read the tests and they look good. I'll review the algorithm changes in the next few days, but feel free to commit the patch in the meantime."

dberlin added inline comments.Feb 12 2017, 12:51 PM
lib/Transforms/Utils/PredicateInfo.cpp
124–128 ↗(On Diff #88089)

Yes, i believe in this refactored version, we can just shove this in the tie.
Thanks for noticing!

Just to note, on the thread about this, Nuno said:
"I just read the tests and they look good. I'll review the algorithm changes in the next few days, but feel free to commit the patch in the meantime."

Go for it then :)

This revision was automatically updated to reflect the committed changes.