Page MenuHomePhabricator

[InstCombine] fix wrong undef handling when converting select to shuffle
ClosedPublic

Authored by spatel on Apr 12 2017, 9:25 AM.

Details

Summary

As discussed in:
https://bugs.llvm.org/show_bug.cgi?id=32486
...the canonicalization of vector select to shufflevector does not hold up when undef elements are present in the condition vector.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel created this revision.Apr 12 2017, 9:25 AM
efriedma added inline comments.Apr 12 2017, 10:31 AM
docs/LangRef.rst
7077 ↗(On Diff #94982)

The second operand may be undef if performing a shuffle from only one vector.

This is technically true, but not really a useful statement. Probably better to explicitly state that an element of the result is undef iff the chosen element of the input is undef.

lib/Transforms/InstCombine/InstCombineSelect.cpp
1059 ↗(On Diff #94982)

You could treat the undef as if it were true or false here (just pick arbitrarily), but I'm not sure what the right heuristic would be. You can just leave it for now, I guess.

spatel updated this revision to Diff 94999.Apr 12 2017, 11:03 AM
spatel marked an inline comment as done.

Patch updated:
Replace a low value statement about shuffle undef-ness in the LangRef.

lib/Transforms/InstCombine/InstCombineSelect.cpp
1059 ↗(On Diff #94982)

Right - I thought about that as a follow-up. We can handle the case where all defined elements are coming from one side or the other by choosing the undef elements from the same side. That could be done in instsimplify, so all we're left with here is the case where we are choosing from both sides and have undef elements.

This revision is now accepted and ready to land.Apr 12 2017, 11:14 AM
This revision was automatically updated to reflect the committed changes.