[PowerPC] Do not attempt to reuse load for 64-bit FP_TO_UINT without FPCVT

[nemanjai]

Summary

We call the function that attempts to reuse the conversion without checking whether the target matches the constraints that the callee expects. This patch adds the check prior to the call.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=43976

[sfertile]

run clang-format, but otherwise for fixing the assertion it LGTM. FWIW I'm not sure the code we produce for the conversion handles negative numbers correctly. For the reduced IR I posted I get:

# %bb.0:                                # %entry
        addis 3, 2, .LCPI0_0@toc@ha
        li 4, 1
        lfs 0, .LCPI0_0@toc@l(3)
        sldi 4, 4, 63
        fsub 2, 1, 0
        fctidz 2, 2
        stfd 2, -8(1)
        fctidz 2, 1
        stfd 2, -16(1)
        ld 3, -8(1)
        ld 5, -16(1)
        fcmpu 0, 1, 0
        xor 3, 3, 4
        bc 12, 0, .LBB0_1
        b .LBB0_2
.LBB0_1:                                # %entry
        addi 3, 5, 0
.LBB0_2:                                # %entry
        std 3, -24(1)
        lfd 0, -24(1)
        fcfid 1, 0
        blr

Which I think mishandles negative values. I cant produce the same output on powerpc64le so I haven't tested it yet, but will try on AIX64 to see if the assembly works like I think it does.

llvm/test/CodeGen/PowerPC/pr43976.ll

4

We can reduce the test further and still exercise the assertion:

define dso_local double @b(double) local_unnamed_addr {
entry:
  %conv = fptoui double %0 to i64
  %conv1 = sitofp i64 %conv to double
  ret double %conv1
}

[sfertile]

Which I think mishandles negative values.

I was wrong. Thanks Nemanja for testing it out on a BE machine for me.

[pkubaj]

Is there anything preventing this from being committed?
If you can, please also backport for 10.0.1.
Thanks!

[nemanjai]

llvm/test/CodeGen/PowerPC/pr43976.ll

4

I initially reduced this down to something like that, but chose not to go ahead with that for two reasons:

1. The test case in the PR is simple enough and I wanted the test case to actually represent the reported test
2. I am not sure if some DAG combine in the future might fold the operations into something different

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

 // the same chain operand as O, we create a token factor from the chain results
 // of O and L, and we replace all uses of O's chain result with that token
 // factor (see spliceIntoChain below for this last part).
 bool PPCTargetLowering::canReuseLoadAddress(SDValue Op, EVT MemVT,
                                             ReuseLoadInfo &RLI,
                                             SelectionDAG &DAG,
                                             ISD::LoadExtType ET) const {
   SDLoc dl(Op);
+  bool ValidFPToUint = Op.getOpcode() == ISD::FP_TO_UINT &&
+    (Subtarget.hasFPCVT() || Op.getValueType() == MVT::i32);
   if (ET == ISD::NON_EXTLOAD &&
-      (Op.getOpcode() == ISD::FP_TO_UINT ||
-       Op.getOpcode() == ISD::FP_TO_SINT) &&
+      (ValidFPToUint || Op.getOpcode() == ISD::FP_TO_SINT) &&
       isOperationLegalOrCustom(Op.getOpcode(),
                                Op.getOperand(0).getValueType())) {
 
     LowerFP_TO_INTForReuse(Op, RLI, DAG, dl);
     return true;
   }
 
   LoadSDNode *LD = dyn_cast<LoadSDNode>(Op);
   if (!LD || LD->getExtensionType() != ET || LD->isVolatile() ||
       LD->isNonTemporal())
     return false;
   if (LD->getMemoryVT() != MemVT)
     return false;
 
   RLI.Ptr = LD->getBasePtr();
   if (LD->isIndexed() && !LD->getOffset().isUndef()) {
     assert(LD->getAddressingMode() == ISD::PRE_INC &&
            "Non-pre-inc AM on PPC?");
     RLI.Ptr = DAG.getNode(ISD::ADD, dl, RLI.Ptr.getValueType(), RLI.Ptr,
                           LD->getOffset());
   }
 
   RLI.Chain = LD->getChain();
   RLI.MPI = LD->getPointerInfo();
   RLI.IsDereferenceable = LD->isDereferenceable();
   RLI.IsInvariant = LD->isInvariant();
   RLI.Alignment = LD->getAlign();
   RLI.AAInfo = LD->getAAInfo();
   RLI.Ranges = LD->getRanges();

llvm/test/CodeGen/PowerPC/pr43976.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=powerpc64-unknown-unknown -verify-machineinstrs \
+; RUN:   -ppc-asm-full-reg-names < %s | FileCheck %s
+@a = dso_local local_unnamed_addr global double 0.000000e+00, align 8
+
+define dso_local signext i32 @b() local_unnamed_addr #0 {
+; CHECK-LABEL: b:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    mflr r0
+; CHECK-NEXT:    std r0, 16(r1)
+; CHECK-NEXT:    stdu r1, -144(r1)
+; CHECK-NEXT:    addis r3, r2, a@toc@ha
+; CHECK-NEXT:    addis r4, r2, .LCPI0_0@toc@ha
+; CHECK-NEXT:    lfd f0, a@toc@l(r3)
+; CHECK-NEXT:    lfs f1, .LCPI0_0@toc@l(r4)
+; CHECK-NEXT:    li r4, 1
+; CHECK-NEXT:    sldi r4, r4, 63
+; CHECK-NEXT:    fsub f2, f0, f1
+; CHECK-NEXT:    fctidz f2, f2
+; CHECK-NEXT:    stfd f2, 128(r1)
+; CHECK-NEXT:    fctidz f2, f0
+; CHECK-NEXT:    stfd f2, 120(r1)
+; CHECK-NEXT:    ld r3, 128(r1)
+; CHECK-NEXT:    ld r5, 120(r1)
+; CHECK-NEXT:    fcmpu cr0, f0, f1
+; CHECK-NEXT:    xor r3, r3, r4
+; CHECK-NEXT:    bc 12, lt, .LBB0_1
+; CHECK-NEXT:    b .LBB0_2
+; CHECK-NEXT:  .LBB0_1: # %entry
+; CHECK-NEXT:    addi r3, r5, 0
+; CHECK-NEXT:  .LBB0_2: # %entry
+; CHECK-NEXT:    std r3, 112(r1)
+; CHECK-NEXT:    addis r3, r2, .LCPI0_1@toc@ha
+; CHECK-NEXT:    lfd f0, 112(r1)
+; CHECK-NEXT:    lfs f1, .LCPI0_1@toc@l(r3)
+; CHECK-NEXT:    fcfid f0, f0
+; CHECK-NEXT:    fmul f0, f0, f1
+; CHECK-NEXT:    fctiwz f0, f0
+; CHECK-NEXT:    stfd f0, 136(r1)
+; CHECK-NEXT:    lwa r3, 140(r1)
+; CHECK-NEXT:    bl g
+; CHECK-NEXT:    nop
+; CHECK-NEXT:    addi r1, r1, 144
+; CHECK-NEXT:    ld r0, 16(r1)
+; CHECK-NEXT:    mtlr r0
+; CHECK-NEXT:    blr
+entry:
+  %0 = load double, double* @a, align 8
+  %conv = fptoui double %0 to i64
+  %conv1 = sitofp i64 %conv to double
+  %mul = fmul double %conv1, 1.000000e+06
+  %conv2 = fptosi double %mul to i32
+  %call = tail call signext i32 @g(i32 signext %conv2) #0
+  ret i32 %call
+}
+
+declare signext i32 @g(i32 signext) local_unnamed_addr
+
+attributes #0 = { nounwind }