[PATCH] D43398: [InstCombine] allow fdiv folds with less than fully 'fast' ops

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: efriedma, wristow, MatzeB, hfinkel.
Herald added a subscriber: mcrosier.

These folds raise 2 questions for me as I'm trying to eliminate the use of 'isFast' here:

1. Do we need both 'arcp' and 'reassoc' to do this? My understanding is that -freciprocal-math allows this:

(X / Y) / Z --> X * (1.0 / Y) * (1.0 / Z)

...which wouldn't be useful here in InstCombine, but that might be a target-specific transform for the DAG.
If we are allowed to reassociate, then we reduce the above as:
X * (1.0 / Y) * (1.0 / Z) --> X * (1.0 / (Y * Z)) --> (X * 1.0) / (Y * Z) --> X / (Y * Z)

...but gcc currently says we can do this with just -freciprocal-math:
https://godbolt.org/g/vx7JnL

Should we allow this with just 'arcp'?

2. We're applying the intersected FMF to the new fmul op. That results in strict fmul ops in the regression tests. Is that too conservative? Strict ops inhibit further reassociation and might cause more FMF removal in early-cse (because that also does FMF intersection).


https://reviews.llvm.org/D43398

Files:
  lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
  test/Transforms/InstCombine/fdiv.ll


Index: test/Transforms/InstCombine/fdiv.ll
===================================================================
--- test/Transforms/InstCombine/fdiv.ll
+++ test/Transforms/InstCombine/fdiv.ll
@@ -119,25 +119,25 @@
 
 define float @div_with_div_numerator(float %x, float %y, float %z) {
 ; CHECK-LABEL: @div_with_div_numerator(
-; CHECK-NEXT:    [[TMP1:%.*]] = fmul ninf float [[Y:%.*]], [[Z:%.*]]
-; CHECK-NEXT:    [[DIV2:%.*]] = fdiv fast float [[X:%.*]], [[TMP1]]
+; CHECK-NEXT:    [[TMP1:%.*]] = fmul float [[Y:%.*]], [[Z:%.*]]
+; CHECK-NEXT:    [[DIV2:%.*]] = fdiv reassoc arcp float [[X:%.*]], [[TMP1]]
 ; CHECK-NEXT:    ret float [[DIV2]]
 ;
   %div1 = fdiv ninf float %x, %y
-  %div2 = fdiv fast float %div1, %z
+  %div2 = fdiv arcp reassoc float %div1, %z
   ret float %div2
 }
 
 ; Z / (X / Y) --> (Z * Y) / X
 
 define <2 x float> @div_with_div_denominator(<2 x float> %x, <2 x float> %y, <2 x float> %z) {
 ; CHECK-LABEL: @div_with_div_denominator(
-; CHECK-NEXT:    [[TMP1:%.*]] = fmul nnan <2 x float> [[Y:%.*]], [[Z:%.*]]
-; CHECK-NEXT:    [[DIV2:%.*]] = fdiv fast <2 x float> [[TMP1]], [[X:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = fmul <2 x float> [[Y:%.*]], [[Z:%.*]]
+; CHECK-NEXT:    [[DIV2:%.*]] = fdiv reassoc arcp <2 x float> [[TMP1]], [[X:%.*]]
 ; CHECK-NEXT:    ret <2 x float> [[DIV2]]
 ;
   %div1 = fdiv nnan <2 x float> %x, %y
-  %div2 = fdiv fast <2 x float> %z, %div1
+  %div2 = fdiv arcp reassoc <2 x float> %z, %div1
   ret <2 x float> %div2
 }
 
Index: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -1424,7 +1424,7 @@
     return nullptr;
   }
 
-  if (AllowReassociate) {
+  if (I.hasAllowReassoc() && I.hasAllowReciprocal()) {
     Value *X, *Y;
     if (match(Op0, m_OneUse(m_FDiv(m_Value(X), m_Value(Y)))) &&
         (!isa<Constant>(Y) || !isa<Constant>(Op1))) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43398.134653.patch
Type: text/x-patch
Size: 1985 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180216/378db218/attachment.bin>