From 6f6bce9783015e86cce87cbdb66c243bb0c0b8f9 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Tue, 27 Oct 2015 20:27:25 +0000 Subject: [PATCH] Use the 'arcp' fast-math-flag when combining repeated FP divisors This is a usage of the IR-level fast-math-flags now that they are propagated to SDNodes. This was originally part of D8900. Removing the global 'enable-unsafe-fp-math' checks will require auto-upgrade and possibly other changes. Differential Revision: http://reviews.llvm.org/D9708 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@251450 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 16 ++++-- test/CodeGen/X86/fdiv-combine.ll | 69 ++++++++++++++++++++---- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 957aa7bf90b..31f0df99d47 100644 --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -8477,7 +8477,9 @@ SDValue DAGCombiner::visitFMA(SDNode *N) { // FDIVs may be lower than the cost of one FDIV and two FMULs. Another reason // is the critical path is increased from "one FDIV" to "one FDIV + one FMUL". SDValue DAGCombiner::combineRepeatedFPDivisors(SDNode *N) { - if (!DAG.getTarget().Options.UnsafeFPMath) + bool UnsafeMath = DAG.getTarget().Options.UnsafeFPMath; + const SDNodeFlags *Flags = N->getFlags(); + if (!UnsafeMath && !Flags->hasAllowReciprocal()) return SDValue(); // Skip if current node is a reciprocal. @@ -8496,9 +8498,14 @@ SDValue DAGCombiner::combineRepeatedFPDivisors(SDNode *N) { // Find all FDIV users of the same divisor. // Use a set because duplicates may be present in the user list. SetVector Users; - for (auto *U : N1->uses()) - if (U->getOpcode() == ISD::FDIV && U->getOperand(1) == N1) - Users.insert(U); + for (auto *U : N1->uses()) { + if (U->getOpcode() == ISD::FDIV && U->getOperand(1) == N1) { + // This division is eligible for optimization only if global unsafe math + // is enabled or if this division allows reciprocal formation. + if (UnsafeMath || U->getFlags()->hasAllowReciprocal()) + Users.insert(U); + } + } // Now that we have the actual number of divisor uses, make sure it meets // the minimum threshold specified by the target. @@ -8508,7 +8515,6 @@ SDValue DAGCombiner::combineRepeatedFPDivisors(SDNode *N) { EVT VT = N->getValueType(0); SDLoc DL(N); SDValue FPOne = DAG.getConstantFP(1.0, DL, VT); - const SDNodeFlags *Flags = &cast(N)->Flags; SDValue Reciprocal = DAG.getNode(ISD::FDIV, DL, VT, FPOne, N1, Flags); // Dividend / Divisor -> Dividend * Reciprocal diff --git a/test/CodeGen/X86/fdiv-combine.ll b/test/CodeGen/X86/fdiv-combine.ll index b65e9d01ab8..d9d9ac401fb 100644 --- a/test/CodeGen/X86/fdiv-combine.ll +++ b/test/CodeGen/X86/fdiv-combine.ll @@ -1,9 +1,11 @@ ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mcpu=x86-64 | FileCheck %s -; Anything more than one division using a single divisor operand +; More than one 'arcp' division using a single divisor operand ; should be converted into a reciprocal and multiplication. -define float @div1_arcp(float %x, float %y, float %z) #0 { +; Don't do anything for just one division. + +define float @div1_arcp(float %x, float %y, float %z) { ; CHECK-LABEL: div1_arcp: ; CHECK: # BB#0: ; CHECK-NEXT: divss %xmm1, %xmm0 @@ -12,13 +14,15 @@ define float @div1_arcp(float %x, float %y, float %z) #0 { ret float %div1 } -define float @div2_arcp(float %x, float %y, float %z) #0 { -; CHECK-LABEL: div2_arcp: +; All math instructions are 'arcp', so optimize. + +define float @div2_arcp_all(float %x, float %y, float %z) { +; CHECK-LABEL: div2_arcp_all: ; CHECK: # BB#0: ; CHECK-NEXT: movss {{.*#+}} xmm3 = mem[0],zero,zero,zero ; CHECK-NEXT: divss %xmm2, %xmm3 -; CHECK-NEXT: mulss %xmm1, %xmm0 ; CHECK-NEXT: mulss %xmm3, %xmm0 +; CHECK-NEXT: mulss %xmm1, %xmm0 ; CHECK-NEXT: mulss %xmm3, %xmm0 ; CHECK-NEXT: retq %div1 = fdiv arcp float %x, %z @@ -27,10 +31,57 @@ define float @div2_arcp(float %x, float %y, float %z) #0 { ret float %div2 } +; The first division is not 'arcp', so do not optimize. + +define float @div2_arcp_partial1(float %x, float %y, float %z) { +; CHECK-LABEL: div2_arcp_partial1: +; CHECK: # BB#0: +; CHECK-NEXT: divss %xmm2, %xmm0 +; CHECK-NEXT: mulss %xmm1, %xmm0 +; CHECK-NEXT: divss %xmm2, %xmm0 +; CHECK-NEXT: retq + %div1 = fdiv float %x, %z + %mul = fmul arcp float %div1, %y + %div2 = fdiv arcp float %mul, %z + ret float %div2 +} + +; The second division is not 'arcp', so do not optimize. + +define float @div2_arcp_partial2(float %x, float %y, float %z) { +; CHECK-LABEL: div2_arcp_partial2: +; CHECK: # BB#0: +; CHECK-NEXT: divss %xmm2, %xmm0 +; CHECK-NEXT: mulss %xmm1, %xmm0 +; CHECK-NEXT: divss %xmm2, %xmm0 +; CHECK-NEXT: retq + %div1 = fdiv arcp float %x, %z + %mul = fmul arcp float %div1, %y + %div2 = fdiv float %mul, %z + ret float %div2 +} + +; The multiply is not 'arcp', but that does not prevent optimizing the divisions. + +define float @div2_arcp_partial3(float %x, float %y, float %z) { +; CHECK-LABEL: div2_arcp_partial3: +; CHECK: # BB#0: +; CHECK-NEXT: movss {{.*#+}} xmm3 = mem[0],zero,zero,zero +; CHECK-NEXT: divss %xmm2, %xmm3 +; CHECK-NEXT: mulss %xmm3, %xmm0 +; CHECK-NEXT: mulss %xmm1, %xmm0 +; CHECK-NEXT: mulss %xmm3, %xmm0 +; CHECK-NEXT: retq + %div1 = fdiv arcp float %x, %z + %mul = fmul float %div1, %y + %div2 = fdiv arcp float %mul, %z + ret float %div2 +} + ; If the reciprocal is already calculated, we should not ; generate an extra multiplication by 1.0. -define double @div3_arcp(double %x, double %y, double %z) #0 { +define double @div3_arcp(double %x, double %y, double %z) { ; CHECK-LABEL: div3_arcp: ; CHECK: # BB#0: ; CHECK-NEXT: movsd{{.*#+}} xmm2 = mem[0],zero @@ -44,7 +95,7 @@ define double @div3_arcp(double %x, double %y, double %z) #0 { ret double %ret } -define void @PR24141() #0 { +define void @PR24141() { ; CHECK-LABEL: PR24141: ; CHECK: callq ; CHECK-NEXT: divsd @@ -57,11 +108,9 @@ while.body: %call = call { double, double } @g(double %x.0) %xv0 = extractvalue { double, double } %call, 0 %xv1 = extractvalue { double, double } %call, 1 - %div = fdiv double %xv0, %xv1 + %div = fdiv arcp double %xv0, %xv1 br label %while.body } declare { double, double } @g(double) -; FIXME: If the backend understands 'arcp', then this attribute is unnecessary. -attributes #0 = { "unsafe-fp-math"="true" } -- 2.34.1