Page MenuHomePhabricator

[InstCombine] extend matchSelectFromAndOr() to work with i1 scalar types
ClosedPublic

Authored by spatel on Jun 26 2016, 5:31 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel updated this revision to Diff 61921.Jun 26 2016, 5:31 PM
spatel retitled this revision from to [InstCombine] extend matchSelectFromAndOr() to work with i1 scalar types.
spatel updated this object.
spatel added reviewers: majnemer, eli.friedman.
spatel added a subscriber: llvm-commits.
eli.friedman edited edge metadata.Jun 29 2016, 4:50 PM

Would it make sense to add "m_SExtOrBool" or something like that, which matches a value which matches either a sext or a value of boolean type? That seems like a cleaner way to generalize the transform.

If the boolean comes from an icmp, it seems likely that you'll end up with ((X == Y) & C) | ((X != Y) & D). I guess you can address that in a followup, if necessary.

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
1654 ↗(On Diff #61921)

Please don't get rid of this comment... you don't have any others which actually describe the transform in full.

Would it make sense to add "m_SExtOrBool" or something like that, which matches a value which matches either a sext or a value of boolean type? That seems like a cleaner way to generalize the transform.

Not sure if that matcher would be generally useful. For the non-splat vectors scenarios that I'm seeing, I may need to add something to ValueTracking like 'isAllOnesOrAllZerosElts()' [return true if a scalar value is -1 or 0 or if all elements of a vector are -1 or 0 (but not necessarily that all elements are the same). So that would be an even more general way to catch these. I'd prefer to keep this patch simpler and then see what's needed for the 'TODO' comment.

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
1654 ↗(On Diff #61921)

Oops - that was an unintended delete. Let me restore it.

spatel updated this revision to Diff 62322.Jun 29 2016, 5:50 PM
spatel edited edge metadata.

Patch updated:
Restore mistakenly deleted comment that describes the transform.

This revision was automatically updated to reflect the committed changes.