Page MenuHomePhabricator

[AArch64] implement GPR (U/S)(MIN/MAX) instruction SDag support
ClosedPublic

Authored by stuij on Nov 28 2022, 6:34 AM.

Details

Summary

Using SelectionDag, lower umin, umax, smin, smax intrinsics to corresponding
UMIN, UMAX, SMIN, SMAX instructions when feat CSSC is available.

See specs for corresponding immediate and register versions in:
https://developer.arm.com/documentation/ddi0602/2022-09/Base-Instructions/

Diff Detail

Event Timeline

stuij created this revision.Nov 28 2022, 6:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 28 2022, 6:34 AM
stuij requested review of this revision.Nov 28 2022, 6:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 28 2022, 6:34 AM
ktkachov added inline comments.
llvm/test/CodeGen/AArch64/min-max.ll
142–150

I'm not very familiar with tests in LLVM, but you shouldn't need CSSC for the vector smax/smin. CSSC only adds GP-register variants.

stuij added inline comments.Nov 28 2022, 7:06 AM
llvm/test/CodeGen/AArch64/min-max.ll
142–150

Thanks for the comment :)
To be verbose: the CHECK-ISEL-CSSC prefix corresponds to a test run of this file with a particular configuration. This file also includes tests for the vector variants of the instructions and tests those with other configurations (specified by te RUN lines at the top of the file). What we're testing here is that the scalar codegen doesn't interfere with the vector codegen.

One Nit, otherwise I'm happy.

llvm/test/CodeGen/AArch64/min-max.ll
2–4

You might get better test check merging with this instead, which would show the cases where CSSC matters and where it doesn't better.

Most of the reason that GlobalISel isn't merging is because it emits the max/min in the opposite order sometimes :(.

lenary accepted this revision.Nov 28 2022, 10:06 AM

(To be clear, you don't need to act on my nit)

This revision is now accepted and ready to land.Nov 28 2022, 10:06 AM
Matt added a subscriber: Matt.Nov 28 2022, 1:12 PM
stuij marked an inline comment as done.Dec 5 2022, 5:46 AM
stuij added inline comments.
llvm/test/CodeGen/AArch64/min-max.ll
2–4

I'll look into this when the gisel patch for this lands.

stuij updated this revision to Diff 480413.Dec 6 2022, 2:54 AM
stuij marked an inline comment as done.

solve minor merge conflict

This revision was landed with ongoing or failed builds.Dec 6 2022, 2:58 AM
This revision was automatically updated to reflect the committed changes.