This is an archive of the discontinued LLVM Phabricator instance.

[InterleaveAccess] Check that binop shuffles have an undef second operand
ClosedPublic

Authored by dmgreen on Mar 31 2023, 1:13 AM.

Details

Summary

It is expected that shuffles that we hoist through binops only have a single vector operand, the other being undef/poison. The checks for isDeInterleaveMaskOfFactor check that all the elements come from inside the first vector, but with non-canonical shuffles the second operand could still have a value. Add a quick check to make sure it is UndefValue as expected, to make sure we don't run into problems with BinOpShuffles not being BinOps.

Fixes #61749

Diff Detail

Event Timeline

dmgreen created this revision.Mar 31 2023, 1:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2023, 1:13 AM
dmgreen requested review of this revision.Mar 31 2023, 1:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2023, 1:13 AM
Peter accepted this revision.EditedMar 31 2023, 3:44 AM

lgtm

One Minor question, what if we have 'shuffle undef, binop, mask', does that mean we don't optimize for such cases?

This revision is now accepted and ready to land.Mar 31 2023, 3:44 AM

lgtm

One Minor question, what if we have 'shuffle undef, binop, mask', does that mean we don't optimize for such cases?

Thanks - Good question, I will make sure there is a test for it. We would expect that to be transformed by instcombine into shuffle binop, undef, mask prior to this pass. If the code had not been canonicalized then we shouldn't do the transform (which will be what happens with this patch, as it will check the second operand is undef).