llvm dev - Jun 2013 - [LLVMdev] ExpandDivRemLibCall vs. AEABI

Folks,

I'm working on bug 16387:
"clang doesn't produce ARM EABI-compliant modulo runtime function"
http://llvm.org/bugs/show_bug.cgi?id=16387

And I need some pointers.

I've changed ARMISelLowering::ARMTargetLowering::ARMTargetLowering() to
associate __aeabi_idivmod variants to RTLIB::{U,S}DIVREM_* library calls,
but now I need to teach the expansion that on AEABI case, the remainder is
not on the stack, but on registers.

However, SelectionDAGLegalize::ExpandDivRemLibCall() assumes the remainder
is *always* on the stack, as is the case for GNU:

  // Remainder is loaded back from the stack frame.
  SDValue Rem = DAG.getLoad(RetVT, dl, CallInfo.second, FIPtr,
                            MachinePointerInfo(), false, false, false, 0);

Since I don't want to add a target-specific condition there, and I found no
hierarchy for DAGLegalize, I'm wondering what's the best approach.

Two options could be:
 * Creating a feature (HasDivRemRegister or whatever) and hard-code AEABI
together with the hard-coded GNU
 * Have some call-back mechanism, possibly upon a flag
(HasSpecialDivRemLowering), and update the remainder result

Both are ugly... :(

Is there anything like that elsewhere, that would hopefully be implemented
inside ARM's tree, that I could follow?

cheers,
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130621/b76d8625/attachment.html>

Hi Renato,
>  * Have some call-back mechanism, possibly upon a flag
> (HasSpecialDivRemLowering), and update the remainder result
If you setOperationAction on SDIVREM and UDIVREM to Custom you can
expand the rtlib call appropriately yourself. There's precedent for
sincos on Darwin systems (both ARM and x86) and in AArch64 for
basically every operation on fp128.

Cheers.

Tim.

On 21 June 2013 15:56, Tim Northover <t.p.northover at gmail.com> wrote:
> If you setOperationAction on SDIVREM and UDIVREM to Custom you can
> expand the rtlib call appropriately yourself. There's precedent for
> sincos on Darwin systems (both ARM and x86) and in AArch64 for
> basically every operation on fp128.
>
I was trying to avoid Custom, the code duplication will not be small... :(

cheers,
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130621/9c0a6066/attachment.html>

On 21 June 2013 15:56, Tim Northover <t.p.northover at gmail.com> wrote:
> If you setOperationAction on SDIVREM and UDIVREM to Custom you can
> expand the rtlib call appropriately yourself.

Tim,

Just following up.

I have assigned Custom lowering:

    setOperationAction(ISD::SREM,  MVT::i32, Custom);
    setOperationAction(ISD::UREM,  MVT::i32, Custom);
    setOperationAction(ISD::SDIVREM, MVT::i32, Custom);
    setOperationAction(ISD::UDIVREM, MVT::i32, Custom);

Added it to the LowerOperation() switch and created a LowerDIVREM(SDValue,
DAG).

My idea is to change the DAG this way:

* {U,S}REM:
  * up to 32bits: call {U,S}DIVREM result in R1 (Follow through or use MOV)
  * 64bits: call {U,S}DIVREM + result in {R2,R3} (Follow through or use MOV)
* {U,S}DIVREM:
  * Change the reminder from stack to to R1 / {R2,R3}

But for that, SelectionDAGLegalize::ExpandDivRemLibCall() has to stop
always loading the result from the stack, and that should become a
post-processing thing. I'm not sure how to change the result at the end,
but I saw that some calls use ARMTargetLowering::ReplaceNodeResults(),
would that be the right place for adding my magic?

Does this make sense? Or is it better for me to copy&paste most of the
ExpandDivRemLibCall() into LowerDIVREM()?

cheers,
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130624/164f925a/attachment.html>