This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen][SVE] Replace deprecated calls in getCopyFromPartsVector()
ClosedPublic

Authored by hkao13 on May 29 2020, 11:35 AM.

Details

Summary

Replaced getVectorNumElements() with getVectorElementCount(). Added operator overloads for class ElementCount. Fixes warning in several AArch64 unit tests.

Diff Detail

Event Timeline

hkao13 created this revision.May 29 2020, 11:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 29 2020, 11:35 AM
efriedma added inline comments.May 29 2020, 12:34 PM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
454

Changing the assertions here is fine.

490

There are significant functional implications here, related to the type breakdown stuff Sander is looking at... a v1i8 is actually one element, an nxv1i8 is not. This needs some real design discussion and testing.

efriedma added inline comments.May 29 2020, 12:39 PM
llvm/include/llvm/Support/TypeSize.h
53 ↗(On Diff #267311)

These operators are a little dubious; it's possible for both A<B and A>=B to be false, which goes against mathematical intuition.

hkao13 added inline comments.May 29 2020, 2:04 PM
llvm/include/llvm/Support/TypeSize.h
53 ↗(On Diff #267311)

Thanks for the review. Strict weak ordering is implemented -- (A < B) and (A >=B) can both be false as long as (A > B) is true. Is there a case where scalable and unscalable widths would be compared? If so then I can remove the LHS and RHS scalable comparisons from the operator overloads.

efriedma added inline comments.May 29 2020, 2:21 PM
llvm/include/llvm/Support/TypeSize.h
53 ↗(On Diff #267311)

I think I'd prefer to just write out the operation in the users for now, until we have some practical experience with what patterns we end up using in practice.

hkao13 updated this revision to Diff 267400.May 29 2020, 3:58 PM

Removed ElementCount operator overloads and reverted getVectorElementCount() for case i8 -> <1 x i1>

@efriedma My weak ordering was wrong. The logic I posted above does not always hold. A > B and B < A can both be true if one is scalable and the other is not. Anyhow what you propose makes sense. I made the changes in the latest edit.

hkao13 marked an inline comment as done.May 29 2020, 4:04 PM
hkao13 added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
490

I reverted this back to getVectorNumElements(), we can revisit this part in a later patch.

efriedma added inline comments.Jun 1 2020, 12:51 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
467

This has the same issues as the other check; it treats an nxv1i8 as having "one" element, and I'm not sure that's appropriate here.

hkao13 updated this revision to Diff 267596.Jun 1 2020, 6:17 AM
hkao13 marked an inline comment as done.
hkao13 added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
467

I reverted this change as well. Thanks.

This revision is now accepted and ready to land.Jun 1 2020, 10:27 AM
This revision was automatically updated to reflect the committed changes.