Page MenuHomePhabricator

[InstCombine] don't change the size of a select if it would mismatch its condition operands' sizes
ClosedPublic

Authored by spatel on May 21 2018, 2:26 PM.

Details

Summary

Don't always:
cast (select (cmp x, y), z, C) --> select (cmp x, y), (cast z), C'

This is something that came up as far back as D26556, and I lost track of it. I suspect that this transform is part of the underlying problem that is inspiring many of the recent proposals that seek to match larger patterns that include a cast op.

If this looks ok, then I can also add the canonicalization in the other direction, so we actively try to get the compare and select to match sizes.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel created this revision.May 21 2018, 2:26 PM
lebedev.ri accepted this revision.May 21 2018, 2:39 PM

Looks sane to me, but please do wait for someone else to review this too.

lib/Transforms/InstCombine/InstCombineCasts.cpp
289–290 ↗(On Diff #147872)
This revision is now accepted and ready to land.May 21 2018, 2:39 PM
spatel added a subscriber: opaparo.

@opaparo can you review this patch? I'm wondering if this will help any of the motivating cases for D46380.

opaparo accepted this revision.May 28 2018, 9:05 AM

@opaparo can you review this patch? I'm wondering if this will help any of the motivating cases for D46380.

It's actually not very helpful for the motivating cases for D46380.
Regardless, the proposed functionality seems optimization-wise correct.

This revision was automatically updated to reflect the committed changes.