[PATCH] D65898: [InstCombine] x /c fabs(x) -> copysign(1.0, x)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a subscriber: lebedev.ri.
spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1246
+  if (I.hasNoNaNs() && I.hasNoInfs() &&
+      match(Op0, m_Intrinsic<Intrinsic::fabs>(m_Specific(Op1)))) {
+    Value *V = Builder.CreateBinaryIntrinsic(
----------------
xbolva00 wrote:
> I tried
> 
>  match(&I, m_FDiv(m_Value(X), m_Intrinsic<Intrinsic::fabs>(m_Specific(X)))) and it failed to match.. uh.
> 
> (this blocked me to possibly introduce m_c_FDiv)
You need to use m_Deferred() for that case when matching commutatively (cc @lebedev.ri ).
But I don't think it's worth adding a commutative matcher for fdiv since it isn't a commutative operation. Not too many cases like this where it can be used?


================
Comment at: test/Transforms/InstCombine/fabs-copysign.ll:21-29
+define double @fabs_copysign_commuted(double %x) {
+; CHECK-LABEL: @fabs_copysign_commuted(
+; CHECK-NEXT:    [[TMP1:%.*]] = call nnan ninf double @llvm.copysign.f64(double 1.000000e+00, double [[X:%.*]])
+; CHECK-NEXT:    ret double [[TMP1]]
+;
+  %f = tail call double @llvm.fabs.f64(double %x)
+  %div = fdiv nnan ninf double %f, %x
----------------
Please push the new tests with baseline CHECKs as a preliminary step.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65898/new/

https://reviews.llvm.org/D65898