This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE][VLS] Move extends into arguments of comparisons
ClosedPublic

Authored by DavidTruby on Jan 25 2022, 6:02 AM.

Details

Summary

When a comparison is extended and it would be free to extend the
arguments to that comparison, we can propagate the extend into those arguments.
This prevents extra instructions being generated to extend the result of the
comparison, which is not free to extend.

This is a resubmission of D116812 with fixes that need another review.
Changed since the previous commit:
The transform is now restricted to VLS to avoid issues with i1 vectors being
illegal there but legal on VLA.
Zero extends are used when the comparison is unsigned.

Diff Detail

Event Timeline

DavidTruby created this revision.Jan 25 2022, 6:02 AM
DavidTruby requested review of this revision.Jan 25 2022, 6:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 25 2022, 6:02 AM

Fix warnings related to switch statement.

paulwalker-arm added inline comments.Jan 25 2022, 8:04 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
15396

Perhaps I'm being pedantic but do you mean "cheap" or do you mean "zero cost"?

15418–15428

ISDOpcode.h has the helper function isSignedIntSetCC()

Matt added a subscriber: Matt.Jan 25 2022, 3:24 PM

Use isSignedIntSetCC to check which extend to perform.
Add tests for signed and unsigned comparisons.

peterwaller-arm accepted this revision.Jan 26 2022, 7:07 AM
peterwaller-arm added inline comments.
llvm/test/CodeGen/AArch64/sve-fixed-length-masked-loads.ll
841

Unimportant: Maybe worth a comment separator just above here to indicate that what follows is different. I rank this of low importance because the function name does cover it already.

868

Nit: differing indentation.

This revision is now accepted and ready to land.Jan 26 2022, 7:07 AM
DavidTruby added inline comments.Jan 26 2022, 7:34 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
15396

I suppose I wasn't confident enough to declare that extending loads were definitely the same cost as normal loads :). Happy to change the name if you think isFreeToExtend would be better

This revision was landed with ongoing or failed builds.Jan 28 2022, 6:16 AM
This revision was automatically updated to reflect the committed changes.