Page MenuHomePhabricator

[X86][AARCH64] Improve ISD::ABS support
ClosedPublic

Authored by RKSimon on Thu, Jan 10, 6:10 AM.

Details

Summary

This patch takes some of the code from D49837 to allow us to enable ISD::ABS support for all SSE vector types.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Thu, Jan 10, 6:10 AM
RKSimon updated this revision to Diff 181048.Thu, Jan 10, 6:43 AM

Add SSE41+ ABS(vXi64 X) optimization using VPBLENDVPD(X, 0-X, X), I can split this off as a separate commit if people prefer.

efriedma added inline comments.Thu, Jan 10, 12:58 PM
test/CodeGen/AArch64/iabs.ll
46 ↗(On Diff #181048)

This sequence is legal, but it isn't actually a good idea; the new sequence is one instruction longer, and has much higher latency.

AArch64TargetLowering::LowerINTRINSIC_WO_CHAIN should probably generate a v1i64 ISD::ABS, not an i64 ISD::ABS.

RKSimon added inline comments.Thu, Jan 10, 1:14 PM
test/CodeGen/AArch64/iabs.ll
46 ↗(On Diff #181048)

Do you want me to add that change to this patch?

RKSimon updated this revision to Diff 181156.Thu, Jan 10, 2:15 PM
RKSimon edited the summary of this revision. (Show Details)

Removed AARCH64 i64 legal ABS

efriedma added inline comments.Thu, Jan 10, 2:30 PM
test/CodeGen/AArch64/iabs.ll
46 ↗(On Diff #181048)

Probably should be a separate patch. I'll try to take care of it soon.

@efriedma I've reduced the aarch64 side to just the test codegen change. I think that should be enough to let this patch continue and any aarch64 side perf tweaks can be done in parallel?

Yes, that's fine.

@craig.topper Are you happy with the X86 side of things?

craig.topper added inline comments.Fri, Jan 11, 1:33 PM
lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
758 ↗(On Diff #181156)

Need to check ISD::ABS in the switch in VectorLegalizer::LegalizeOp that controls the call to TLI.getOperationAction. Otherwise its not considered a vector op and gets delayed to LegalizeDAG.

RKSimon updated this revision to Diff 181427.Fri, Jan 11, 11:56 PM

rebase and add VectorLegalizer::LegalizeOp case

This revision is now accepted and ready to land.Sat, Jan 12, 12:03 AM
This revision was automatically updated to reflect the committed changes.