Page MenuHomePhabricator

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

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

Diff Detail


Event Timeline

RKSimon created this revision.Jan 10 2019, 6:10 AM
RKSimon updated this revision to Diff 181048.Jan 10 2019, 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.Jan 10 2019, 12:58 PM
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.Jan 10 2019, 1:14 PM
46 ↗(On Diff #181048)

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

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

Removed AARCH64 i64 legal ABS

efriedma added inline comments.Jan 10 2019, 2:30 PM
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.Jan 11 2019, 1:33 PM
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.Jan 11 2019, 11:56 PM

rebase and add VectorLegalizer::LegalizeOp case

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