[ValueTracking] Take PHI's poison-generating flags into account #161530

dtcxzyw · 2025-10-01T14:50:14Z

ninf/nnan in the phi node may produce poison values. They should be considered in isGuaranteedNotToBeUndefOrPoison.
Closes #161524.

llvmbot · 2025-10-01T14:50:59Z

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

ninf/nnan in the phi node may produce poison values. They should be considered in isGuaranteedNotToBeUndefOrPoison.
Closes #161524.

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

2 Files Affected:

(modified) llvm/lib/Analysis/ValueTracking.cpp (+18-17)
(modified) llvm/test/Transforms/InstCombine/freeze-phi.ll (+28)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 6f11b250cf21f..09a8fbea065ac 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -7651,25 +7651,26 @@ static bool isGuaranteedNotToBeUndefOrPoison(
         return true;
     }
 
-    if (const auto *PN = dyn_cast<PHINode>(V)) {
-      unsigned Num = PN->getNumIncomingValues();
-      bool IsWellDefined = true;
-      for (unsigned i = 0; i < Num; ++i) {
-        if (PN == PN->getIncomingValue(i))
-          continue;
-        auto *TI = PN->getIncomingBlock(i)->getTerminator();
-        if (!isGuaranteedNotToBeUndefOrPoison(PN->getIncomingValue(i), AC, TI,
-                                              DT, Depth + 1, Kind)) {
-          IsWellDefined = false;
-          break;
+    if (!::canCreateUndefOrPoison(Opr, Kind,
+                                  /*ConsiderFlagsAndMetadata=*/true)) {
+      if (const auto *PN = dyn_cast<PHINode>(V)) {
+        unsigned Num = PN->getNumIncomingValues();
+        bool IsWellDefined = true;
+        for (unsigned i = 0; i < Num; ++i) {
+          if (PN == PN->getIncomingValue(i))
+            continue;
+          auto *TI = PN->getIncomingBlock(i)->getTerminator();
+          if (!isGuaranteedNotToBeUndefOrPoison(PN->getIncomingValue(i), AC, TI,
+                                                DT, Depth + 1, Kind)) {
+            IsWellDefined = false;
+            break;
+          }
         }
-      }
-      if (IsWellDefined)
+        if (IsWellDefined)
+          return true;
+      } else if (all_of(Opr->operands(), OpCheck))
         return true;
-    } else if (!::canCreateUndefOrPoison(Opr, Kind,
-                                         /*ConsiderFlagsAndMetadata*/ true) &&
-               all_of(Opr->operands(), OpCheck))
-      return true;
+    }
   }
 
   if (auto *I = dyn_cast<LoadInst>(V))
diff --git a/llvm/test/Transforms/InstCombine/freeze-phi.ll b/llvm/test/Transforms/InstCombine/freeze-phi.ll
index cdc9a5efe5933..62bb9dc31b76b 100644
--- a/llvm/test/Transforms/InstCombine/freeze-phi.ll
+++ b/llvm/test/Transforms/InstCombine/freeze-phi.ll
@@ -212,3 +212,31 @@ D:
   %y.fr = freeze i32 %y
   ret i32 %y.fr
 }
+
+; Make sure that fmf in phi node is dropped when freeze get folded.
+
+define float @pr161524(float noundef %arg) {
+; CHECK-LABEL: @pr161524(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[COND:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[ARG:%.*]], i32 144)
+; CHECK-NEXT:    br i1 [[COND]], label [[IF_THEN:%.*]], label [[IF_EXIT:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[FADD:%.*]] = fadd float [[ARG]], 1.000000e+00
+; CHECK-NEXT:    br label [[IF_EXIT]]
+; CHECK:       if.exit:
+; CHECK-NEXT:    [[RET:%.*]] = phi float [ [[FADD]], [[IF_THEN]] ], [ [[ARG]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    ret float [[RET]]
+;
+entry:
+  %cond = tail call i1 @llvm.is.fpclass.f32(float %arg, i32 144)
+  br i1 %cond, label %if.then, label %if.exit
+
+if.then:
+  %fadd = fadd float %arg, 1.0
+  br label %if.exit
+
+if.exit:
+  %ret = phi ninf float [ %fadd, %if.then ], [ %arg, %entry ]
+  %ret.fr = freeze float %ret
+  ret float %ret.fr
+}

llvmbot · 2025-10-01T14:51:00Z

@llvm/pr-subscribers-llvm-analysis

Author: Yingwei Zheng (dtcxzyw)

Changes

ninf/nnan in the phi node may produce poison values. They should be considered in isGuaranteedNotToBeUndefOrPoison.
Closes #161524.

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

2 Files Affected:

(modified) llvm/lib/Analysis/ValueTracking.cpp (+18-17)
(modified) llvm/test/Transforms/InstCombine/freeze-phi.ll (+28)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 6f11b250cf21f..09a8fbea065ac 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -7651,25 +7651,26 @@ static bool isGuaranteedNotToBeUndefOrPoison(
         return true;
     }
 
-    if (const auto *PN = dyn_cast<PHINode>(V)) {
-      unsigned Num = PN->getNumIncomingValues();
-      bool IsWellDefined = true;
-      for (unsigned i = 0; i < Num; ++i) {
-        if (PN == PN->getIncomingValue(i))
-          continue;
-        auto *TI = PN->getIncomingBlock(i)->getTerminator();
-        if (!isGuaranteedNotToBeUndefOrPoison(PN->getIncomingValue(i), AC, TI,
-                                              DT, Depth + 1, Kind)) {
-          IsWellDefined = false;
-          break;
+    if (!::canCreateUndefOrPoison(Opr, Kind,
+                                  /*ConsiderFlagsAndMetadata=*/true)) {
+      if (const auto *PN = dyn_cast<PHINode>(V)) {
+        unsigned Num = PN->getNumIncomingValues();
+        bool IsWellDefined = true;
+        for (unsigned i = 0; i < Num; ++i) {
+          if (PN == PN->getIncomingValue(i))
+            continue;
+          auto *TI = PN->getIncomingBlock(i)->getTerminator();
+          if (!isGuaranteedNotToBeUndefOrPoison(PN->getIncomingValue(i), AC, TI,
+                                                DT, Depth + 1, Kind)) {
+            IsWellDefined = false;
+            break;
+          }
         }
-      }
-      if (IsWellDefined)
+        if (IsWellDefined)
+          return true;
+      } else if (all_of(Opr->operands(), OpCheck))
         return true;
-    } else if (!::canCreateUndefOrPoison(Opr, Kind,
-                                         /*ConsiderFlagsAndMetadata*/ true) &&
-               all_of(Opr->operands(), OpCheck))
-      return true;
+    }
   }
 
   if (auto *I = dyn_cast<LoadInst>(V))
diff --git a/llvm/test/Transforms/InstCombine/freeze-phi.ll b/llvm/test/Transforms/InstCombine/freeze-phi.ll
index cdc9a5efe5933..62bb9dc31b76b 100644
--- a/llvm/test/Transforms/InstCombine/freeze-phi.ll
+++ b/llvm/test/Transforms/InstCombine/freeze-phi.ll
@@ -212,3 +212,31 @@ D:
   %y.fr = freeze i32 %y
   ret i32 %y.fr
 }
+
+; Make sure that fmf in phi node is dropped when freeze get folded.
+
+define float @pr161524(float noundef %arg) {
+; CHECK-LABEL: @pr161524(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[COND:%.*]] = tail call i1 @llvm.is.fpclass.f32(float [[ARG:%.*]], i32 144)
+; CHECK-NEXT:    br i1 [[COND]], label [[IF_THEN:%.*]], label [[IF_EXIT:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[FADD:%.*]] = fadd float [[ARG]], 1.000000e+00
+; CHECK-NEXT:    br label [[IF_EXIT]]
+; CHECK:       if.exit:
+; CHECK-NEXT:    [[RET:%.*]] = phi float [ [[FADD]], [[IF_THEN]] ], [ [[ARG]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    ret float [[RET]]
+;
+entry:
+  %cond = tail call i1 @llvm.is.fpclass.f32(float %arg, i32 144)
+  br i1 %cond, label %if.then, label %if.exit
+
+if.then:
+  %fadd = fadd float %arg, 1.0
+  br label %if.exit
+
+if.exit:
+  %ret = phi ninf float [ %fadd, %if.then ], [ %arg, %entry ]
+  %ret.fr = freeze float %ret
+  ret float %ret.fr
+}

nikic

LGTM

nikic · 2025-10-01T14:58:49Z

llvm/lib/Analysis/ValueTracking.cpp

-          IsWellDefined = false;
-          break;
+    if (!::canCreateUndefOrPoison(Opr, Kind,
+                                  /*ConsiderFlagsAndMetadata=*/true)) {


I think I'd prefer just checking ->hasPoisonGeneratingFlags() in this case.

Oh, I didn't notice that you pulled out the condition from the general Operator code below, and it's not phi specific. With that in mind, I like your original code more. Sorry about that.

This reverts commit 7363500.

…#161530) ninf/nnan in the phi node may produce poison values. They should be considered in `isGuaranteedNotToBeUndefOrPoison`. Closes llvm#161524.

dtcxzyw added 2 commits October 1, 2025 22:31

[InstCombine] Add pre-commit tests. NFC.

78a1575

[ValueTracking] Take PHI's poison-generating flags into account

7e71b3f

dtcxzyw requested a review from arsenm October 1, 2025 14:50

dtcxzyw requested a review from nikic as a code owner October 1, 2025 14:50

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

nikic approved these changes Oct 1, 2025

View reviewed changes

dtcxzyw added 2 commits October 2, 2025 00:11

[ValueTracking] Address review comments.

7363500

Revert "[ValueTracking] Address review comments."

2c37f58

This reverts commit 7363500.

nikic approved these changes Oct 1, 2025

View reviewed changes

dtcxzyw merged commit 57a9f79 into llvm:main Oct 1, 2025
7 of 9 checks passed

dtcxzyw deleted the fix-161524 branch October 1, 2025 17:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ValueTracking] Take PHI's poison-generating flags into account #161530

[ValueTracking] Take PHI's poison-generating flags into account #161530

dtcxzyw commented Oct 1, 2025

Uh oh!

llvmbot commented Oct 1, 2025

Uh oh!

llvmbot commented Oct 1, 2025

Uh oh!

nikic left a comment

Uh oh!

nikic Oct 1, 2025

Uh oh!

nikic Oct 1, 2025

Uh oh!

Uh oh!

Uh oh!

[ValueTracking] Take PHI's poison-generating flags into account #161530

[ValueTracking] Take PHI's poison-generating flags into account #161530

Conversation

dtcxzyw commented Oct 1, 2025

Uh oh!

llvmbot commented Oct 1, 2025

Uh oh!

llvmbot commented Oct 1, 2025

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

nikic Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

nikic Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!