[llvm-dev] Bug: Library functions for ISD::SRA, ISD::SHL, and ISD::SRL

[Eli Friedman via llvm-dev]

All in-tree targets have variable shift instructions for native integer types, except for AVR.  And AVR implements custom lowering.  I’m not sure what else would be required to actually make marking a shift as “libcall” actually work well; technically, the change you’re proposing might produce valid code, but it would be terrible quality in a lot of cases.  So I’m not eager to add partial support just for out-of-tree targets.

Marking CTLZ_ZERO_UNDEF as “LibCall” was implemented in https://reviews.llvm.org/D47917 .  Probably straightforward to extend that to cover CTTZ_ZERO_UNDEF and CTPOP.

-Eli

From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Joan Lluch via llvm-dev
Sent: Monday, June 10, 2019 8:31 AM
To: llvm-dev <llvm-dev at lists.llvm.org>
Subject: [EXT] [llvm-dev] Bug: Library functions for ISD::SRA, ISD::SHL, and ISD::SRL

LLVM appears to support Library functions for ISD::SRA ,ISD::SHL, and ISD::SRL, as they are properly defined in RuntimeLibCalls.def.

The library functions defined in RuntimeLibCalls.def (among others) are these:

HANDLE_LIBCALL(SRA_I16, "__ashrhi3")
HANDLE_LIBCALL(SRA_I32, "__ashrsi3")
HANDLE_LIBCALL(SRA_I64, "__ashrdi3")

However, setting

setOperationAction(ISD::SRA, MVT::i16, LibCall);

in the TargetLowering constructor causes LLVM to stop with an assert as the shift instruction can’t be selected.

The problem is in SelectionDAGLegalize::ConvertNodeToLibcall because there’s no switch case for any of the Shift instructions.

The problem gets solved by just adding switch cases like this (and similar for the other shift instructions):

case ISD::SRA:
      Results.push_back(ExpandIntLibCall(Node, false,
                                       RTLIB::SRA_I16,
                                       RTLIB::SRA_I16, RTLIB::SRA_I32,
                                       RTLIB::SRA_I64, RTLIB::SRA_I128));

I think this is a BUG by omission of necessary switch cases.


Similarly, the following ISD codes  ISD::CTTZ, ISD::CTLZ, ISD::CTPOP do not define any Library calls, despite LLVM being able to fully expand them into rather large code for targets that do not natively implement them.
I regard this also as an omission/bug, because not all targets would benefit from the custom expansion of these ISD codes, which as said can get unnecessarily long and costly.
Instead, LLVM should have them available as possible LibCalls.

Any comments or opinions on these subjects are appreciated

Joan Lluch


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190610/cef3869f/attachment.html>