[InstSimplify] Missed optimization in math expression: squashing exp(log), log(exp)

[Quolyk]

Summary

This patch enables folding following expressions under -ffast-math flag: exp(log(x)) -> x, exp2(log2(x)) -> x, log(exp(x)) -> x, log2(exp2(x)) -> x

[hfinkel]

LGTM

[Quolyk]

I don't have commit access, please commit.

[davide]

I don't have access to my checkout here, but I'm fairly sure we do something similar in SimplifyLibCalls.
I understand many of the pattern proposed will be duplicated, which is, not ideal.
Also, I'm pretty sure @scanon once taught me (when I implemented a similar transformation in simplifylibcalls) that these can underflow/overflow pretty dramatically, so, shouldn't these only be enabled under -ffast-math ?

[Quolyk]

In D41381#960534, @davide wrote:

I don't have access to my checkout here, but I'm fairly sure we do something similar in SimplifyLibCalls.
I understand many of the pattern proposed will be duplicated, which is, not ideal.
Also, I'm pretty sure @scanon once taught me (when I implemented a similar transformation in simplifylibcalls) that these can underflow/overflow pretty dramatically, so, shouldn't these only be enabled under -ffast-math ?

These transforms are only enabled under -ffast-math, II->isFast() checks here for it. Tests explicitly show correctness. SimlifyLibCalls doesn't have these optimizations, but I can move transforms to that file if it's necessary.

[davide]

Sorry, I clearly misread your previous diff. LGTM.

[spatel]

Since this hasn't been committed yet, let me ask: why are we doing this in instcombine rather than instsimplify? When no new instructions are created, we generally want that transform in InstSimplify because that makes passes like GVN more powerful. InstSimplify already has "SimplifyIntrinsic", so I think this could fit neatly in there.

[hfinkel]

In D41381#965101, @spatel wrote:

Since this hasn't been committed yet, let me ask: why are we doing this in instcombine rather than instsimplify? When no new instructions are created, we generally want that transform in InstSimplify because that makes passes like GVN more powerful. InstSimplify already has "SimplifyIntrinsic", so I think this could fit neatly in there.

Indeed, my oversight. Yes, it would be good to have this in InstSimplify.

[Quolyk]

Moved to InstructionSimplify. This patch changed a bit, ping me if ok, I'll commit.

[spatel]

What is the expected result for this kind of test (please add at least one test like this)?

define double @exp_log_partly_fast(double %a) {
  %1 = call double @llvm.log.f64(double %a)          ; strict
  %2 = call fast double @llvm.exp.f64(double %1)
  ret double %2
}

[Quolyk]

Updated tests

[spatel]

LGTM - see inline comments for some potential improvements.

lib/Analysis/InstructionSimplify.cpp

4520

I don't see much value in naming this local (could just use *ArgBegin in the line below here).

So you could shrink this down to:

Value *X;
if (Q.CxtI->isFast() && match(*ArgBegin, m_Intrinsic<Intrinsic::log>(m_Value(X))))
  return X;
return nullptr;

4521

As I've said in other reviews, I think explicitly setting this to nullptr is just noise.

test/Transforms/InstSimplify/exp-intrinsic.ll

5–6

It's a matter of taste, but since these are highly similar transforms, I'd put all of the tests together in one test file to match the fact that they're all handled together in one block of code.

28–46

Looking back at the discussion in D5584, I think this is correct. Only the later op needs to be relaxed to do the transform.

[Quolyk]

@spatel I updated diff, please take a look.

[spatel]

LGTM - thanks for the clean-up.

lib/Analysis/InstructionSimplify.cpp

if (NumOperands == 1) {
     case Intrinsic::bitreverse: {
       Value *IIOperand = *ArgBegin;
       Value *X = nullptr;
       // bitreverse(bitreverse(x)) -> x
       if (match(IIOperand, m_BitReverse(m_Value(X))))
         return X;
       return nullptr;
     }
+    case Intrinsic::exp: {
+      if (Q.CxtI->isFast()) {
+        Value *IIOperand = *ArgBegin;
+        Value *X = nullptr;
+        // exp(log(x)) -> x
+        if (match(IIOperand, m_Intrinsic<Intrinsic::log>(m_Value(X))))
+          return X;
+      }
+      return nullptr;
+    }
+    case Intrinsic::exp2: {
+      if (Q.CxtI->isFast()) {
+        Value *IIOperand = *ArgBegin;
+        Value *X = nullptr;
+        // exp2(log2(x)) -> x
+        if (match(IIOperand, m_Intrinsic<Intrinsic::log2>(m_Value(X))))
+          return X;
+      }
+      return nullptr;
+    }
+    case Intrinsic::log: {
+      if (Q.CxtI->isFast()) {
+        Value *IIOperand = *ArgBegin;
+        Value *X = nullptr;
+        // log(exp(x)) -> x
+        if (match(IIOperand, m_Intrinsic<Intrinsic::exp>(m_Value(X))))
+          return X;
+      }
+      return nullptr;
+    }
+    case Intrinsic::log2: {
+      if (Q.CxtI->isFast()) {
+        Value *IIOperand = *ArgBegin;
+        Value *X = nullptr;
+        // log2(exp2(x)) -> x
+        if (match(IIOperand, m_Intrinsic<Intrinsic::exp2>(m_Value(X))))
+          return X;
+      }
+      return nullptr;
+    }
     default:
       return nullptr;
     }
   }
 
   // Binary Ops
   if (NumOperands == 2) {
     Value *LHS = *ArgBegin;

test/Transforms/InstSimplify/exp-intrinsic.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instsimplify -S | FileCheck %s
+
+declare double @llvm.exp.f64(double)
+declare double @llvm.log.f64(double)
+
+define double @exp_log(double %a) {
+; CHECK-LABEL: @exp_log(
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.log.f64(double [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = call double @llvm.exp.f64(double [[TMP1]])
+; CHECK-NEXT:    ret double [[TMP2]]
+;
+  %1 = call double @llvm.log.f64(double %a)
+  %2 = call double @llvm.exp.f64(double %1)
+  ret double %2
+}
+
+define double @exp_log_fast(double %a) {
+; CHECK-LABEL: @exp_log_fast(
+; CHECK-NEXT:    ret double [[A:%.*]]
+;
+  %1 = call fast double @llvm.log.f64(double %a)
+  %2 = call fast double @llvm.exp.f64(double %1)
+  ret double %2
+}
+
+define double @exp_log_exp_log(double %a) {
+; CHECK-LABEL: @exp_log_exp_log(
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.log.f64(double [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = call double @llvm.exp.f64(double [[TMP1]])
+; CHECK-NEXT:    [[TMP3:%.*]] = call double @llvm.log.f64(double [[TMP2]])
+; CHECK-NEXT:    [[TMP4:%.*]] = call double @llvm.exp.f64(double [[TMP3]])
+; CHECK-NEXT:    ret double [[TMP4]]
+;
+  %1 = call double @llvm.log.f64(double %a)
+  %2 = call double @llvm.exp.f64(double %1)
+  %3 = call double @llvm.log.f64(double %2)
+  %4 = call double @llvm.exp.f64(double %3)
+  ret double %4
+}
+
+define double @exp_log_exp_log_fast(double %a) {
+; CHECK-LABEL: @exp_log_exp_log_fast(
+; CHECK-NEXT:    ret double [[A:%.*]]
+;
+  %1 = call fast double @llvm.log.f64(double %a)
+  %2 = call fast double @llvm.exp.f64(double %1)
+  %3 = call fast double @llvm.log.f64(double %2)
+  %4 = call fast double @llvm.exp.f64(double %3)
+  ret double %4
+}

test/Transforms/InstSimplify/exp2-intrinsic.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instsimplify -S | FileCheck %s
+
+declare double @llvm.exp2.f64(double)
+declare double @llvm.log2.f64(double)
+
+define double @exp2_log2(double %a) {
+; CHECK-LABEL: @exp2_log2(
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.log2.f64(double [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = call double @llvm.exp2.f64(double [[TMP1]])
+; CHECK-NEXT:    ret double [[TMP2]]
+;
+  %1 = call double @llvm.log2.f64(double %a)
+  %2 = call double @llvm.exp2.f64(double %1)
+  ret double %2
+}
+
+define double @exp2_log2_fast(double %a) {
+; CHECK-LABEL: @exp2_log2_fast(
+; CHECK-NEXT:    ret double [[A:%.*]]
+;
+  %1 = call fast double @llvm.log2.f64(double %a)
+  %2 = call fast double @llvm.exp2.f64(double %1)
+  ret double %2
+}
+
+define double @exp2_log2_exp2_log2(double %a) {
+; CHECK-LABEL: @exp2_log2_exp2_log2(
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.log2.f64(double [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = call double @llvm.exp2.f64(double [[TMP1]])
+; CHECK-NEXT:    [[TMP3:%.*]] = call double @llvm.log2.f64(double [[TMP2]])
+; CHECK-NEXT:    [[TMP4:%.*]] = call double @llvm.exp2.f64(double [[TMP3]])
+; CHECK-NEXT:    ret double [[TMP4]]
+;
+  %1 = call double @llvm.log2.f64(double %a)
+  %2 = call double @llvm.exp2.f64(double %1)
+  %3 = call double @llvm.log2.f64(double %2)
+  %4 = call double @llvm.exp2.f64(double %3)
+  ret double %4
+}
+
+define double @exp2_log2_exp2_log2_fast(double %a) {
+; CHECK-LABEL: @exp2_log2_exp2_log2_fast(
+; CHECK-NEXT:    ret double [[A:%.*]]
+;
+  %1 = call fast double @llvm.log2.f64(double %a)
+  %2 = call fast double @llvm.exp2.f64(double %1)
+  %3 = call fast double @llvm.log2.f64(double %2)
+  %4 = call fast double @llvm.exp2.f64(double %3)
+  ret double %4
+}

test/Transforms/InstSimplify/log-intrinsic.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instsimplify -S | FileCheck %s
+
+declare double @llvm.log.f64(double)
+declare double @llvm.exp.f64(double)
+
+define double @log_exp(double %a) {
+; CHECK-LABEL: @log_exp(
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.exp.f64(double [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = call double @llvm.log.f64(double [[TMP1]])
+; CHECK-NEXT:    ret double [[TMP2]]
+;
+  %1 = call double @llvm.exp.f64(double %a)
+  %2 = call double @llvm.log.f64(double %1)
+  ret double %2
+}
+
+define double @log_exp_fast(double %a) {
+; CHECK-LABEL: @log_exp_fast(
+; CHECK-NEXT:    ret double [[A:%.*]]
+;
+  %1 = call fast double @llvm.exp.f64(double %a)
+  %2 = call fast double @llvm.log.f64(double %1)
+  ret double %2
+}
+
+define double @log_exp_log_exp(double %a) {
+; CHECK-LABEL: @log_exp_log_exp(
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.exp.f64(double [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = call double @llvm.log.f64(double [[TMP1]])
+; CHECK-NEXT:    [[TMP3:%.*]] = call double @llvm.exp.f64(double [[TMP2]])
+; CHECK-NEXT:    [[TMP4:%.*]] = call double @llvm.log.f64(double [[TMP3]])
+; CHECK-NEXT:    ret double [[TMP4]]
+;
+  %1 = call double @llvm.exp.f64(double %a)
+  %2 = call double @llvm.log.f64(double %1)
+  %3 = call double @llvm.exp.f64(double %2)
+  %4 = call double @llvm.log.f64(double %3)
+  ret double %4
+}
+
+define double @log_exp_log_exp_fast(double %a) {
+; CHECK-LABEL: @log_exp_log_exp_fast(
+; CHECK-NEXT:    ret double [[A:%.*]]
+;
+  %1 = call fast double @llvm.exp.f64(double %a)
+  %2 = call fast double @llvm.log.f64(double %1)
+  %3 = call fast double @llvm.exp.f64(double %2)
+  %4 = call fast double @llvm.log.f64(double %3)
+  ret double %4
+}

test/Transforms/InstSimplify/log2-intrinsic.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instsimplify -S | FileCheck %s
+
+declare double @llvm.log2.f64(double)
+declare double @llvm.exp2.f64(double)
+
+define double @log2_exp2(double %a) {
+; CHECK-LABEL: @log2_exp2(
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.exp2.f64(double [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = call double @llvm.log2.f64(double [[TMP1]])
+; CHECK-NEXT:    ret double [[TMP2]]
+;
+  %1 = call double @llvm.exp2.f64(double %a)
+  %2 = call double @llvm.log2.f64(double %1)
+  ret double %2
+}
+
+define double @log2_exp2_fast(double %a) {
+; CHECK-LABEL: @log2_exp2_fast(
+; CHECK-NEXT:    ret double [[A:%.*]]
+;
+  %1 = call fast double @llvm.exp2.f64(double %a)
+  %2 = call fast double @llvm.log2.f64(double %1)
+  ret double %2
+}
+
+define double @log2_exp2_log2_exp2(double %a) {
+; CHECK-LABEL: @log2_exp2_log2_exp2(
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.exp2.f64(double [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = call double @llvm.log2.f64(double [[TMP1]])
+; CHECK-NEXT:    [[TMP3:%.*]] = call double @llvm.exp2.f64(double [[TMP2]])
+; CHECK-NEXT:    [[TMP4:%.*]] = call double @llvm.log2.f64(double [[TMP3]])
+; CHECK-NEXT:    ret double [[TMP4]]
+;
+  %1 = call double @llvm.exp2.f64(double %a)
+  %2 = call double @llvm.log2.f64(double %1)
+  %3 = call double @llvm.exp2.f64(double %2)
+  %4 = call double @llvm.log2.f64(double %3)
+  ret double %4
+}
+
+define double @log2_exp2_log2_exp2_fast(double %a) {
+; CHECK-LABEL: @log2_exp2_log2_exp2_fast(
+; CHECK-NEXT:    ret double [[A:%.*]]
+;
+  %1 = call fast double @llvm.exp2.f64(double %a)
+  %2 = call fast double @llvm.log2.f64(double %1)
+  %3 = call fast double @llvm.exp2.f64(double %2)
+  %4 = call fast double @llvm.log2.f64(double %3)
+  ret double %4
+}