llvm dev - Sep 2017 - Unexpected dag combine in arm64

Hi,

There are some generic DAGCombine functions which replace an SDNode with a
newly created but otherwise equivalent SDNode. Specifically, visit for
SINT_TO_FP, UINT_TO_FP, FP_TO_SINT, FP_TO_UINT where the operand is a
constant. This unconditionally prevents target specific dag combines from
executing for these nodes.



The `// fold (fp_to_sint c1fp) -> c1` creates a new node which is
functionally a copy of the input. I’d like to delete this clause to allow a
target specific combine to run.

SDValue DAGCombiner::visitFP_TO_SINT(SDNode *N) {

  SDValue N0 = N->getOperand(0);

  EVT VT = N->getValueType(0);



  // fold (fp_to_sint c1fp) -> c1

  if (isConstantFPBuildVectorOrConstantFP(N0))

    return DAG.getNode(ISD::FP_TO_SINT, SDLoc(N), VT, N0);



  return FoldIntToFPToInt(N, DAG);

}



However fp128-folding.ll and arm64-fp128-folding.ll codegen tests fail if I
do so. Allowing the target specific dag combines to run changes codegen for
constants. Unfortunately, I don’t know arm64. Could someone tell me whether
replacing:



        .text

        .file   "<stdin>"

        .section        .rodata.cst16,"aM", at progbits,16

        .p2align        4               // -- Begin function test_folding

.LCPI0_0:

        .xword  0                       // fp128 42

        .xword  4612899879264452608

        .text

        .globl  test_folding

        .p2align        2

        .type   test_folding, at function

test_folding:                           // @test_folding

        .cfi_startproc

// BB#0:

        sub     sp, sp, #16             // =16

.Lcfi0:

        .cfi_def_cfa_offset 16

       adrp    x8, .LCPI0_0

        ldr     q0, [x8, :lo12:.LCPI0_0]

        mov     w8, #42

        str     w8, [sp, #12]

        add     sp, sp, #16             // =16

        ret

.Lfunc_end0:

        .size   test_folding, .Lfunc_end0-test_folding

        .cfi_endproc

                                        // -- End function



        .section        ".note.GNU-stack","", at progbits



with:



        .text

        .file   "<stdin>"

        .globl  test_folding            // -- Begin function test_folding

        .p2align        2

        .type   test_folding, at function

test_folding:                           // @test_folding

        .cfi_startproc

// BB#0:

        str     x30, [sp, #-16]!        // 8-byte Folded Spill

.Lcfi0:

        .cfi_def_cfa_offset 16

.Lcfi1:

        .cfi_offset w30, -16

        mov     w8, #42

        mov     w0, #42

        str     w8, [sp, #12]

        bl      __floatsitf

        ldr     x30, [sp], #16          // 8-byte Folded Reload

        ret

.Lfunc_end0:

        .size   test_folding, .Lfunc_end0-test_folding

        .cfi_endproc

                                        // -- End function



        .section        ".note.GNU-stack","", at progbits



Is good, bad, irrelevant? I’m assuming ‘bad’ since __floatsitf looks like a
function call. If it is indeed bad, can I tempt anyone from arm to look at
this? Failing that, would a patch against your back end be welcome?



Thanks!



Jon
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170905/1f2c6a68/attachment.html>

On 9/5/2017 4:01 AM, Jon Chesterfield via llvm-dev
wrote:
>
>     Hi,
>
>     There are some generic DAGCombine functions which replace an
>     SDNode with a newly created but otherwise equivalent SDNode.
>     Specifically, visit for SINT_TO_FP, UINT_TO_FP, FP_TO_SINT,
>     FP_TO_UINT where the operand is a constant. This unconditionally
>     prevents target specific dag combines from executing for these nodes.
>
If you want to materialize a floating-point constant using an 
integer-to-floating-point conversion, I would suggest using a 
target-specific node rather than a target-independent one.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

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