[InstCombine] Propagate poison pow[i], [us]add, [us]sub and [us]mul #146750

badumbatish · 2025-07-02T17:38:04Z

Fixes #146560 as well as propagate poison for [us]add, [us]sub and [us]mul

llvmbot · 2025-07-02T17:38:36Z

@llvm/pr-subscribers-llvm-transforms

Author: jjasmine (badumbatish)

Changes

Fixes #146560.

As the issue is about folding with poison and with power operand like 0.0 and 1.0, I didn't add any optimization for folding pow(x, 0) (is it even allowed? I'm not sure)

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

2 Files Affected:

(modified) llvm/lib/Analysis/InstructionSimplify.cpp (+6)
(modified) llvm/test/Transforms/InstSimplify/fold-intrinsics.ll (+31-1)

diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index cb1dae92faf92..c7cbd627117da 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -6673,7 +6673,13 @@ Value *llvm::simplifyBinaryIntrinsic(Intrinsic::ID IID, Type *ReturnType,
       if (auto *C1 = dyn_cast<Constant>(Op1))
         return simplifyRelativeLoad(C0, C1, Q.DL);
     break;
+  case Intrinsic::pow:
+    if (isa<PoisonValue>(Op0) || isa<PoisonValue>(Op1))
+      return PoisonValue::get(ReturnType);
+    break;
   case Intrinsic::powi:
+    if (isa<PoisonValue>(Op0) || isa<PoisonValue>(Op1))
+      return PoisonValue::get(ReturnType);
     if (auto *Power = dyn_cast<ConstantInt>(Op1)) {
       // powi(x, 0) -> 1.0
       if (Power->isZero())
diff --git a/llvm/test/Transforms/InstSimplify/fold-intrinsics.ll b/llvm/test/Transforms/InstSimplify/fold-intrinsics.ll
index e45aa3fd09ce0..1fd47af6946cb 100644
--- a/llvm/test/Transforms/InstSimplify/fold-intrinsics.ll
+++ b/llvm/test/Transforms/InstSimplify/fold-intrinsics.ll
@@ -33,7 +33,7 @@ define void @powi(double %V, ptr%P) {
 define void @powi_i16(float %V, ptr%P) {
 ; CHECK-LABEL: @powi_i16(
 ; CHECK-NEXT:    store volatile float 1.000000e+00, ptr [[P:%.*]], align 4
-; CHECK-NEXT:    store volatile float [[V:%.*]], ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float [[D:%.*]], ptr [[P]], align 4
 ; CHECK-NEXT:    ret void
 ;
   %B = tail call float @llvm.powi.f32.i16(float %V, i16 0) nounwind
@@ -44,3 +44,33 @@ define void @powi_i16(float %V, ptr%P) {
 
   ret void
 }
+
+define void @pow_poison_i16(i16 %arg_int,float %arg_flt, ptr %P) {
+; CHECK-LABEL: @pow_poison_i16(
+; CHECK-NEXT:    store volatile float poison, ptr [[P:%.*]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    ret void
+;
+  %2 = tail call float @llvm.powi.f32.i16(float poison, i16 %arg_int) nounwind
+  store volatile float %2, ptr %P
+
+  %3 = tail call float @llvm.pow(float poison, float %arg_flt) nounwind
+  store volatile float %3, ptr %P
+
+  %4 = tail call float @llvm.powi.f32.i16(float %arg_flt, i16 poison) nounwind
+  store volatile float %4, ptr %P
+
+  %5 = tail call float @llvm.pow(float %arg_flt, float poison) nounwind
+  store volatile float %5, ptr %P
+
+  %6 = tail call float @llvm.powi.f32.i16(float poison, i16 poison) nounwind
+  store volatile float %6, ptr %P
+
+  %7 = tail call float @llvm.pow(float poison, float poison) nounwind
+  store volatile float %7, ptr %P
+  ret void
+}

llvmbot · 2025-07-02T17:38:37Z

@llvm/pr-subscribers-llvm-analysis

Author: jjasmine (badumbatish)

Changes

Fixes #146560.

As the issue is about folding with poison and with power operand like 0.0 and 1.0, I didn't add any optimization for folding pow(x, 0) (is it even allowed? I'm not sure)

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

2 Files Affected:

(modified) llvm/lib/Analysis/InstructionSimplify.cpp (+6)
(modified) llvm/test/Transforms/InstSimplify/fold-intrinsics.ll (+31-1)

diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index cb1dae92faf92..c7cbd627117da 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -6673,7 +6673,13 @@ Value *llvm::simplifyBinaryIntrinsic(Intrinsic::ID IID, Type *ReturnType,
       if (auto *C1 = dyn_cast<Constant>(Op1))
         return simplifyRelativeLoad(C0, C1, Q.DL);
     break;
+  case Intrinsic::pow:
+    if (isa<PoisonValue>(Op0) || isa<PoisonValue>(Op1))
+      return PoisonValue::get(ReturnType);
+    break;
   case Intrinsic::powi:
+    if (isa<PoisonValue>(Op0) || isa<PoisonValue>(Op1))
+      return PoisonValue::get(ReturnType);
     if (auto *Power = dyn_cast<ConstantInt>(Op1)) {
       // powi(x, 0) -> 1.0
       if (Power->isZero())
diff --git a/llvm/test/Transforms/InstSimplify/fold-intrinsics.ll b/llvm/test/Transforms/InstSimplify/fold-intrinsics.ll
index e45aa3fd09ce0..1fd47af6946cb 100644
--- a/llvm/test/Transforms/InstSimplify/fold-intrinsics.ll
+++ b/llvm/test/Transforms/InstSimplify/fold-intrinsics.ll
@@ -33,7 +33,7 @@ define void @powi(double %V, ptr%P) {
 define void @powi_i16(float %V, ptr%P) {
 ; CHECK-LABEL: @powi_i16(
 ; CHECK-NEXT:    store volatile float 1.000000e+00, ptr [[P:%.*]], align 4
-; CHECK-NEXT:    store volatile float [[V:%.*]], ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float [[D:%.*]], ptr [[P]], align 4
 ; CHECK-NEXT:    ret void
 ;
   %B = tail call float @llvm.powi.f32.i16(float %V, i16 0) nounwind
@@ -44,3 +44,33 @@ define void @powi_i16(float %V, ptr%P) {
 
   ret void
 }
+
+define void @pow_poison_i16(i16 %arg_int,float %arg_flt, ptr %P) {
+; CHECK-LABEL: @pow_poison_i16(
+; CHECK-NEXT:    store volatile float poison, ptr [[P:%.*]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    store volatile float poison, ptr [[P]], align 4
+; CHECK-NEXT:    ret void
+;
+  %2 = tail call float @llvm.powi.f32.i16(float poison, i16 %arg_int) nounwind
+  store volatile float %2, ptr %P
+
+  %3 = tail call float @llvm.pow(float poison, float %arg_flt) nounwind
+  store volatile float %3, ptr %P
+
+  %4 = tail call float @llvm.powi.f32.i16(float %arg_flt, i16 poison) nounwind
+  store volatile float %4, ptr %P
+
+  %5 = tail call float @llvm.pow(float %arg_flt, float poison) nounwind
+  store volatile float %5, ptr %P
+
+  %6 = tail call float @llvm.powi.f32.i16(float poison, i16 poison) nounwind
+  store volatile float %6, ptr %P
+
+  %7 = tail call float @llvm.pow(float poison, float poison) nounwind
+  store volatile float %7, ptr %P
+  ret void
+}

badumbatish · 2025-07-02T18:27:24Z

will add vector type into test case

lukel97 · 2025-07-02T19:28:06Z

Is it possible to handle this in ConstantFolding? That way IRBuilder etc. will automatically fold it to poison without the need to run InstSimplify, see: #141821 (review)

badumbatish · 2025-07-02T19:37:48Z

on it

badumbatish · 2025-07-02T23:58:54Z

@lukel97 ok i digged around and prototyped in the codebase a bit, and together with this comment

But I guess we'll still need to figure out how to handle the case whenever not all arguments are constant, e.g only one operand is poison. Should that be handled by ValueTracking?

Yeah. Generally we should handle these cases in propagatesPoison and simplifyIntrinsic...

I think these are not in ConstantFolding at the moment. but in InstructionSimplify.

I tried handling this in ConstantFolding but it was clunky with how the Operands array to be passed in is <Const*>, requiring both arguments to pow[i] to both be poison to be foldable.

Tagging @dtcxzyw for some directions

lukel97

Ah I see what you mean about having a possibly non-constant arg. In that case I think having it in InstSimplify is fine.

LGTM but please wait for another reviewer

llvm/test/Transforms/InstSimplify/fold-intrinsics.ll

nikic

I don't want to add more of these one-off special cases which result in different poison-propagation support between ValueTracking, ConstantFolding and InstSimplify. I've put up #146878 to consolidate ValueTracking and ConstantFolding. We can then do the same for InstSimplify. And then this should just become a matter of adding extra intrinsics to the list.

badumbatish · 2025-07-03T18:40:18Z

rebased and refactored. waiting for your pr to be merged.

Refactor support for pow[i] poison folding by rebasing on Nikic's changes and implement the changes in InstructionSimplify. Fix a few test case and added more Intrinsics to switch case in ValueTracking.

llvm/lib/Analysis/InstructionSimplify.cpp

llvm/lib/Analysis/ValueTracking.cpp

nikic

LGTM

lukel97

LGTM, I think there's a few fold to poison cases in simplifyBinaryIntrinsic can probably be removed afterwards as an NFC

badumbatish requested a review from nikic as a code owner July 2, 2025 17:38

badumbatish changed the title ~~[InstCombine] Propagate poison powi~~ [InstCombine] Propagate poison pow[i] Jul 2, 2025

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Jul 2, 2025

badumbatish mentioned this pull request Jul 2, 2025

[ConstantFolding][Intrinsics] Missed folding of poison in a few trivially vectorizable intrinsics #146769

Open

lukel97 approved these changes Jul 3, 2025

View reviewed changes

llvm/test/Transforms/InstSimplify/fold-intrinsics.ll Show resolved Hide resolved

llvm/test/Transforms/InstSimplify/fold-intrinsics.ll Outdated Show resolved Hide resolved

llvm/test/Transforms/InstSimplify/fold-intrinsics.ll Outdated Show resolved Hide resolved

lukel97 requested a review from dtcxzyw July 3, 2025 10:16

nikic requested changes Jul 3, 2025

View reviewed changes

badumbatish force-pushed the propagate_poison_powi branch from 407b63b to 6fd04bf Compare July 3, 2025 18:39

badumbatish added 7 commits July 4, 2025 09:12

Precommit missed opt for pow poison

61d05be

[InstCombine] Fold poison pow to poison

a67e87e

Refactor support for pow[i] poison folding

1da97c9

Refactor support for pow[i] poison folding by rebasing on Nikic's changes and implement the changes in InstructionSimplify. Fix a few test case and added more Intrinsics to switch case in ValueTracking.

Fix naming nits from PR

69771ea

Remove old one off cse of poison folding

251bfa3

Added vector support for pow[i] poison

e08eb3f

Update unittest for pow[i] poison

d2244eb

badumbatish force-pushed the propagate_poison_powi branch from 8fc29ae to d2244eb Compare July 4, 2025 16:26

lukel97 reviewed Jul 4, 2025

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Show resolved Hide resolved

llvm/lib/Analysis/ValueTracking.cpp Outdated Show resolved Hide resolved

Remove case comments for intrinsics poison

413adc0

nikic approved these changes Jul 4, 2025

View reviewed changes

badumbatish changed the title ~~[InstCombine] Propagate poison pow[i]~~ [InstCombine] Propagate poison pow[i], [us]add, [us]sub and [us]mul Jul 4, 2025

lukel97 approved these changes Jul 4, 2025

View reviewed changes

lukel97 merged commit 07286b1 into llvm:main Jul 4, 2025
9 checks passed

badumbatish deleted the propagate_poison_powi branch July 4, 2025 22:16

badumbatish mentioned this pull request Jul 4, 2025

[NFC] Clean up one off case of poison folding in simplifyBinaryIntrinsic #147116

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[InstCombine] Propagate poison pow[i], [us]add, [us]sub and [us]mul #146750

[InstCombine] Propagate poison pow[i], [us]add, [us]sub and [us]mul #146750

Uh oh!

badumbatish commented Jul 2, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jul 2, 2025

Uh oh!

llvmbot commented Jul 2, 2025

Uh oh!

badumbatish commented Jul 2, 2025

Uh oh!

lukel97 commented Jul 2, 2025

Uh oh!

badumbatish commented Jul 2, 2025

Uh oh!

badumbatish commented Jul 2, 2025 •

edited

Loading

Uh oh!

lukel97 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nikic left a comment

Uh oh!

badumbatish commented Jul 3, 2025

Uh oh!

Uh oh!

Uh oh!

nikic left a comment

Uh oh!

lukel97 left a comment

Uh oh!

Uh oh!

Uh oh!

[InstCombine] Propagate poison pow[i], [us]add, [us]sub and [us]mul #146750

[InstCombine] Propagate poison pow[i], [us]add, [us]sub and [us]mul #146750

Uh oh!

Conversation

badumbatish commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 2, 2025

Uh oh!

llvmbot commented Jul 2, 2025

Uh oh!

badumbatish commented Jul 2, 2025

Uh oh!

lukel97 commented Jul 2, 2025

Uh oh!

badumbatish commented Jul 2, 2025

Uh oh!

badumbatish commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukel97 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

badumbatish commented Jul 3, 2025

Uh oh!

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

lukel97 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

badumbatish commented Jul 2, 2025 •

edited

Loading

badumbatish commented Jul 2, 2025 •

edited

Loading