Page MenuHomePhabricator

[SelectionDAG] Add a signed integer absolute intrinsic
AbandonedPublic

Authored by RKSimon on Nov 7 2016, 10:50 AM.

Details

Summary

This patch introduces a signed integer absolute intrinsic and ISD instruction.

Currently this is only setup for the X86 vector PABS* instructions but other targets should be able to make to use of this as well.

Further pattern matching may be added at the instcombine level in due course and the intrinsic enabled by default but at present the code is only used in the dagcombiner if the operation is legal/custom or the intrinsic is explicitly called.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon updated this revision to Diff 77061.Nov 7 2016, 10:50 AM
RKSimon retitled this revision from to [SelectionDAG] Add a signed integer absolute intrinsic.
RKSimon updated this object.
RKSimon set the repository for this revision to rL LLVM.
RKSimon added a subscriber: llvm-commits.

Please propose the IR intrinsic on llvmdev; it isn't obvious we actually want it.

Moving ABS from the x86-specific opcodes to the general set of SelectionDAG opcodes seems reasonable, given that ABS instructions are pretty common in vector ISAs.

lib/Target/X86/X86ISelLowering.cpp
19775

(No comment, but Phabricator won't let me delete this for some reason.)

jmolloy edited edge metadata.Nov 8 2016, 12:45 AM

I agree with Eli, this should be discussed in an RFC on llvm-dev.

The proposed semantics are quite strange at first sight. I couldn't understand why you'd care that the input is signed and the result is unsigned until I saw the special case for INT_MIN.

What is the purpose of that special case? the usual way of implementing signed integer abs() is v & (UINT_MAX-1) (clearing the sign bit), which of course doesn't work for this case. How do you propose other architectures implement it, and why is it needed?

James

RKSimon abandoned this revision.May 30 2019, 11:20 AM

Abandoning - we have most of this now and use matchSelectPattern to detect ABS patterns instead of introducing a llvm.abs. intrinsic

Herald added a project: Restricted Project. ยท View Herald TranscriptMay 30 2019, 11:20 AM