This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][GlobalISel] Optimise lowering for some vector types for min/max
ClosedPublic

Authored by Rin on Jul 9 2021, 5:56 AM.

Details

Summary

This patch optimises the lowering for some vector types for the min/max instruction.

Diff Detail

Event Timeline

Rin created this revision.Jul 9 2021, 5:56 AM
Rin requested review of this revision.Jul 9 2021, 5:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 9 2021, 5:56 AM
Rin edited the summary of this revision. (Show Details)Jul 9 2021, 5:58 AM
dmgreen added inline comments.Jul 9 2021, 5:59 AM
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
173

I'm not sure you have to clamp the elements on a v2s64, as they are going to be expanded anyway.

Should it be dealing with vXi16 and vXi8 though?

Rin updated this revision to Diff 357915.Jul 12 2021, 6:08 AM

Add optimisation for vXi16 and vXi8 vector types

Rin updated this revision to Diff 357916.Jul 12 2021, 6:08 AM

clang-format

Rin marked an inline comment as done.Jul 12 2021, 6:09 AM
Rin updated this revision to Diff 357933.Jul 12 2021, 7:03 AM

Make min-max.ll test global-isel

aemerson added inline comments.Jul 12 2021, 11:35 AM
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
172

You can use clampMin/MaxNumElts instead to do these.

dmgreen added inline comments.Jul 12 2021, 11:50 AM
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
172

It looks like clampNumElements is a wrapper around clampMinNumElements(..).clampMaxNumElements(..);

Out of interest, what would make the individual methods better to use themselves? Is it something about how small types are widened?

173

It turns out that G_ICMP doesn't support splitting vectors yet, which is why v2s64 is here too. Otherwise it could be lowered.

(It might be worth added a FIXME to that extent.)

aemerson added inline comments.Jul 12 2021, 5:16 PM
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
172

You're right, I had the which-implements-which relation swapped in my head.

Rin updated this revision to Diff 358264.Jul 13 2021, 7:39 AM

Add FIXME comment

Rin marked 4 inline comments as done.Jul 13 2021, 7:40 AM
aemerson accepted this revision.Jul 14 2021, 1:16 PM

LGTM.

This revision is now accepted and ready to land.Jul 14 2021, 1:16 PM