Page MenuHomePhabricator

[EarlyCSE] Propagate conditions of AND and OR instructions
ClosedPublic

Authored by mkazantsev on May 31 2018, 1:18 AM.

Details

Summary

This patches teaches EarlyCSE to figure out that if and i1 %x, %y is true then both
%x and %y are true in the taken branch, and if or i1 %x, %y is false then both
%x and %y are false in non-taken branch. Fix for PR37635.

Diff Detail

Repository
rL LLVM

Event Timeline

mkazantsev created this revision.May 31 2018, 1:18 AM
lebedev.ri added inline comments.
lib/Transforms/Scalar/EarlyCSE.cpp
723 ↗(On Diff #149244)

It would be less confusing to name it BOp

728 ↗(On Diff #149244)

It would be less confusing to name it BOp

mkazantsev marked 2 inline comments as done.
mkazantsev edited the summary of this revision. (Show Details)

If we want to start expanding this, i would suggest just using predicateinfo, which wouldn't require worklisting/etc to achieve this.

I'm not sure if it gives benefits, but the idea is interesting. Do you mind if it goes as a follow-up?

So, the goal would be to reduce the maintenance burden and significantly more complex logic you are starting to introduce here about what can be propagated where.
If it doesn't bring benefits, i'd like to understand why.

I'm fine doing it as a followup.

I will do it as a follow-up then.

reames accepted this revision.Jun 13 2018, 10:32 AM

Current patch LGTM.

As a follow up, I think it might make sense to refactor the interface of the helper a bit. In particular, you have the SimpleValue::canHandle logic both in the caller and the callee. Might make sense to sink it into the callee only and check when taking items out of the worklist.

Danny, I'm not quite clear why PredicateInfo is the right direction to take here? Can you explain? Probably best done on llvm-dev instead of an individual review thread just so we have a wider audience.

This revision is now accepted and ready to land.Jun 13 2018, 10:32 AM
This revision was automatically updated to reflect the committed changes.