[InstCombine] Add missing fold for fsqrt(select(b, c1, c2)) => select(b, fsqrt(c1), fsqrt(c2))

[Rajveer100]

Resolves #110591

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Rajveer Singh Bharadwaj (Rajveer100)

Changes

Resolves #110591

---

Full diff: https://github.com/llvm/llvm-project/pull/113084.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+8)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index c8b9f166b16020..911e5baba8a4de 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1699,6 +1699,14 @@ Instruction *InstCombinerImpl::FoldOpIntoSelect(Instruction &Op, SelectInst *SI,
   if (SI->getType()->isIntOrIntVectorTy(1))
     return nullptr;
 
+  if (auto *II = dyn_cast<IntrinsicInst>(&Op)) {
+    if (II->getIntrinsicID() == Intrinsic::sqrt) {
+      Value *NewTV = Builder.CreateUnaryIntrinsic(Intrinsic::sqrt, TV, nullptr, "sqrt_fold");
+      Value *NewFV = Builder.CreateUnaryIntrinsic(Intrinsic::sqrt, FV, nullptr, "sqrt_fold");
+      return SelectInst::Create(SI->getCondition(), NewTV, NewFV, "", nullptr, SI);
+    }
+  }
+
   // Test if a FCmpInst instruction is used exclusively by a select as
   // part of a minimum or maximum operation. If so, refrain from doing
   // any other folding. This helps out other analyses which understand

[Rajveer100]

@RKSimon
I am not quite sure if this is the intended way as I do see lot of helper functions (like unary, binary, etc) for individual broken down expressions.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[goldsteinn]

Missing tests?
https://llvm.org/docs/InstCombineContributorGuide.html#tests

[goldsteinn]

As this currently exists, it will also fold if one of the arms is not constant.

[arsenm]

Needs tests

[arsenm]

Don't bother naming these values.

You should also be able to preserve at least some of the flags

[RKSimon]

The fold should only occur if at least one of the new sqrt calls will constant fold away.

[Rajveer100]

Quick question:

https://alive2.llvm.org/ce/z/YJkX7m

Isn't this fold already applied in the existing source code?

[arsenm]

Quick question:

https://alive2.llvm.org/ce/z/YJkX7m

Isn't this fold already applied in the existing source code?

Yes, that looks like this is already happening

[Rajveer100]

In that case the original issue and this PR can be closed?

Edit: Issue was closed, hence closing this.