This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Add patterns for signed and unsigned min/max instructions
ClosedPublic

Authored by dancgr on Dec 20 2019, 11:06 AM.

Details

Summary

Add pattern matching for the following instructions:

  • smax, smin, umax, umin

Diff Detail

Event Timeline

dancgr created this revision.Dec 20 2019, 11:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 20 2019, 11:06 AM
dancgr retitled this revision from [AArch64][SVE] Add patterns for some int arith instructions to [AArch64][SVE] Add patterns for signed and unsigned min/max instructions.Dec 20 2019, 11:06 AM

Do we actually need an intrinsic for this, as opposed to adding patterns for ISD::SMAX etc.?

dancgr marked an inline comment as done.Dec 23 2019, 7:05 AM

As far as I know the ISD::SMAX only takes one input (https://llvm.org/docs/LangRef.html#llvm-experimental-vector-reduce-smax-intrinsic), in this case we need two inputs so we can do the max between the input vector and the immediate value.

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
2943

This shouldn't be here.

Wong SMAX; you're referring to VECREDUCE_SMAX, which isn't relevant here. There is no IR intrinsic for ISD::SMAX; it's pattern-matched from select instructions.

dancgr updated this revision to Diff 235174.Dec 23 2019, 12:19 PM

Update to use default ISD:SMAX variety.

Wong SMAX; you're referring to VECREDUCE_SMAX, which isn't relevant here. There is no IR intrinsic for ISD::SMAX; it's pattern-matched from select instructions.

That makes way more sense, I have updated the patch to reflect that.

c-rhodes added inline comments.Dec 30 2019, 2:16 AM
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
2930

Should this be zero extended (getZExtValue)?

llvm/lib/Target/AArch64/SVEInstrFormats.td
215

SVEUArithImmPat?

llvm/test/CodeGen/AArch64/sve-int-arith-imm.ll
9

the BB check can be removed

dancgr marked 3 inline comments as done.Jan 6 2020, 6:49 AM
dancgr added inline comments.
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
2930

If we use getZExtValue this returns an incorrect results when we have i8 values like 129.

llvm/lib/Target/AArch64/SVEInstrFormats.td
215

Will update this

llvm/test/CodeGen/AArch64/sve-int-arith-imm.ll
9

Will be removing that.

dancgr updated this revision to Diff 236599.Jan 7 2020, 8:31 AM

Updated some minor changes.

dancgr marked 2 inline comments as done and an inline comment as not done.Jan 7 2020, 8:32 AM
dancgr added a subscriber: eli.friedman.EditedJan 9 2020, 1:00 PM

@efriedma Any other remarks on this patch?

efriedma accepted this revision.Jan 9 2020, 1:45 PM

LGTM

This revision is now accepted and ready to land.Jan 9 2020, 1:45 PM
This revision was automatically updated to reflect the committed changes.