This is an archive of the discontinued LLVM Phabricator instance.

[DAGTypeLegalizer] Handle ZERO_EXTEND of promoted integer in WidenVecRes_Convert.
ClosedPublic

Authored by jonpa on Aug 20 2020, 1:12 AM.

Details

Summary

On SystemZ, a ZERO_EXTEND of an i1 vector handled by WidenVecRes_Convert() always ended up being scalarized, because the type action of the input is promotion which was previously an unhandled case in this method.

This fixes https://bugs.llvm.org/show_bug.cgi?id=47132.

Patch by Eli Friedman.

Diff Detail

Event Timeline

jonpa created this revision.Aug 20 2020, 1:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 20 2020, 1:12 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
jonpa requested review of this revision.Aug 20 2020, 1:12 AM
jonpa added inline comments.Aug 20 2020, 1:19 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
3331

Does this assume that the final (unroll) part of the method will always be reached? In that case, should that be made explicit with an extra "else" wrapping the cases for Widening/Legal, and alsomaybe an assert that the promoted InVT does then not need Widening or is legal?

uweigand added inline comments.Aug 28 2020, 6:17 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
3331

The result of ZExtPromotedInteger is guaranteed to always have legal type, so afterwards getTypeAction(InVT) will always be TypeWidenLegal.

However, I'm no longer sure all cases are covered here. The situation is that we have a vector zero-extend operation from a type vNiA to another type vNiB (same number of elements, B > A). We know vNiB is not legal, and the preferred legal type is vMiB (with some M > N). If we enter the if, we also know that vNiA is not legal, and the preferred legal type is vNiC (with some C > A).

Therefore, we want to transform the initial operation vNiA -> vNiB into an operation vNiC -> vMiB.

But we do not know anything about the relationship between C and B necessarily. If C is still smaller than B, the operation remains an extension. If C is now larger than B, the operation becomes a truncation. Both these cases are handled. But C could now also be equal to B on some platforms (this cannot happen on Z since all legal vector types have the same total size, but in theory it could happen on a platform that supports multiple legal vector widths). In this case the operation should be neither an extension nor a truncation, just a vector widening. I don't see this handled.

jonpa updated this revision to Diff 289941.Sep 4 2020, 6:56 AM

But we do not know anything about the relationship between C and B necessarily.

Added a check for this.

uweigand accepted this revision.Sep 8 2020, 6:03 AM

This LGTM now. Thanks!

This revision is now accepted and ready to land.Sep 8 2020, 6:03 AM