[ValueTracking] Fix computeKnownFPClass handling of nsz by cardigan1008 · Pull Request #186315 · llvm/llvm-project

cardigan1008 · 2026-03-13T05:00:49Z

This patch fixes incorrect nofpclass inference in ValueTracking for floating-point operations with nsz.

With nsz, +0.0 and -0.0 are interchangeable for the current operation. Update computeKnownFPClass to expand a known single zero sign to include both pzero and nzero under nsz, so later nofpclass inference does not preserve signed-zero-specific facts.

llvmbot · 2026-03-13T05:01:22Z

@llvm/pr-subscribers-llvm-transforms

Author: Yunbo Ni (cardigan1008)

Changes

Fixes #185920

This patch improves nofpclass inference in ValueTracking for floating-point arithmetic with nsz.

When nsz is present, distinctions between +0.0 and -0.0 are not semantically observable, so nofpclass reasoning should avoid deriving sign-sensitive zero properties from such operations.

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

2 Files Affected:

(modified) llvm/lib/Analysis/ValueTracking.cpp (+4-1)
(modified) llvm/test/Transforms/Attributor/nofpclass.ll (+11)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 0672dc889b640..25a8a8166d3af 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -5588,7 +5588,10 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
       DenormalMode Mode =
           F ? F->getDenormalMode(FltSem) : DenormalMode::getDynamic();
 
-      if (Self && Opc == Instruction::FAdd) {
+      const FPMathOperator *FPop = cast<FPMathOperator>(Op);
+      bool HasNSZ = FPop->hasNoSignedZeros();
+
+      if (!HasNSZ && Self && Opc == Instruction::FAdd) {
         Known = KnownFPClass::fadd_self(KnownLHS, Mode);
       } else {
         // RHS is canonically cheaper to compute. Skip inspecting the LHS if
diff --git a/llvm/test/Transforms/Attributor/nofpclass.ll b/llvm/test/Transforms/Attributor/nofpclass.ll
index 507d0f3ff98f3..3946c9f88e41d 100644
--- a/llvm/test/Transforms/Attributor/nofpclass.ll
+++ b/llvm/test/Transforms/Attributor/nofpclass.ll
@@ -3326,6 +3326,17 @@ define float @fadd_double_known_negative_nonsub_dynamic(float noundef nofpclass(
   ret float %add
 }
 
+define float @fadd_double_known_negative_zero_nsz(float noundef nofpclass(ninf pzero sub nnorm) %arg) {
+; CHECK: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define noundef nofpclass(ninf nsub nnorm) float @fadd_double_known_negative_zero_nsz
+; CHECK-SAME: (float noundef nofpclass(ninf pzero sub nnorm) [[ARG:%.*]]) #[[ATTR3]] {
+; CHECK-NEXT:    [[ADD:%.*]] = fadd nsz float [[ARG]], [[ARG]]
+; CHECK-NEXT:    ret float [[ADD]]
+;
+  %add = fadd nsz float %arg, %arg
+  ret float %add
+}
+
 define float @fsub_self(float noundef %arg) {
 ; CHECK: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none)
 ; CHECK-LABEL: define noundef float @fsub_self

llvmbot · 2026-03-13T05:01:23Z

@llvm/pr-subscribers-llvm-analysis

Author: Yunbo Ni (cardigan1008)

Changes

Fixes #185920

This patch improves nofpclass inference in ValueTracking for floating-point arithmetic with nsz.

When nsz is present, distinctions between +0.0 and -0.0 are not semantically observable, so nofpclass reasoning should avoid deriving sign-sensitive zero properties from such operations.

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

2 Files Affected:

(modified) llvm/lib/Analysis/ValueTracking.cpp (+4-1)
(modified) llvm/test/Transforms/Attributor/nofpclass.ll (+11)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 0672dc889b640..25a8a8166d3af 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -5588,7 +5588,10 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
       DenormalMode Mode =
           F ? F->getDenormalMode(FltSem) : DenormalMode::getDynamic();
 
-      if (Self && Opc == Instruction::FAdd) {
+      const FPMathOperator *FPop = cast<FPMathOperator>(Op);
+      bool HasNSZ = FPop->hasNoSignedZeros();
+
+      if (!HasNSZ && Self && Opc == Instruction::FAdd) {
         Known = KnownFPClass::fadd_self(KnownLHS, Mode);
       } else {
         // RHS is canonically cheaper to compute. Skip inspecting the LHS if
diff --git a/llvm/test/Transforms/Attributor/nofpclass.ll b/llvm/test/Transforms/Attributor/nofpclass.ll
index 507d0f3ff98f3..3946c9f88e41d 100644
--- a/llvm/test/Transforms/Attributor/nofpclass.ll
+++ b/llvm/test/Transforms/Attributor/nofpclass.ll
@@ -3326,6 +3326,17 @@ define float @fadd_double_known_negative_nonsub_dynamic(float noundef nofpclass(
   ret float %add
 }
 
+define float @fadd_double_known_negative_zero_nsz(float noundef nofpclass(ninf pzero sub nnorm) %arg) {
+; CHECK: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define noundef nofpclass(ninf nsub nnorm) float @fadd_double_known_negative_zero_nsz
+; CHECK-SAME: (float noundef nofpclass(ninf pzero sub nnorm) [[ARG:%.*]]) #[[ATTR3]] {
+; CHECK-NEXT:    [[ADD:%.*]] = fadd nsz float [[ARG]], [[ARG]]
+; CHECK-NEXT:    ret float [[ADD]]
+;
+  %add = fadd nsz float %arg, %arg
+  ret float %add
+}
+
 define float @fsub_self(float noundef %arg) {
 ; CHECK: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none)
 ; CHECK-LABEL: define noundef float @fsub_self

dtcxzyw

It is a fragile way to fix the issue. nsz is always droppable.

I don't object to the fix. IIRC we have used similar workarounds for nsz in the tree.

nikic

Hm... shouldn't we handle nsz generically somehow? If nsz is set, then for all inputs we need to treat treat a pzero known FP class as also including nzero and vice versa.

arsenm · 2026-03-20T20:00:51Z

Title is misleading? This is fixing a correctness problem, not making the analysis result better?

cardigan1008 · 2026-04-10T09:01:23Z

Title is misleading? This is fixing a correctness problem, not making the analysis result better?

Updated the title and description.

cardigan1008 · 2026-04-10T09:05:21Z

Hm... shouldn't we handle nsz generically somehow? If nsz is set, then for all inputs we need to treat treat a pzero known FP class as also including nzero and vice versa.

Thanks for the suggestion. I've updated the fix to widen the zero with nsz set.

github-actions · 2026-04-10T09:50:27Z

🐧 Linux x64 Test Results

194068 tests passed
5111 tests skipped

✅ The build succeeded and all tests passed.

github-actions · 2026-04-10T09:50:27Z

🪟 Windows x64 Test Results

133771 tests passed
3128 tests skipped

✅ The build succeeded and all tests passed.

arsenm · 2026-04-10T10:08:56Z

+  // With no-signed-zeros semantics, +0 and -0 are interchangeable.
+  // If the operation has nsz and only one sign of zero is possible in the
+  // result, the other must also be considered possible.
+  if (const auto *FPOp = dyn_cast_or_null<FPMathOperator>(Op)) {


This check was already performed above, can you do the flag check in the same place?

It will be late as the existing flag check is before fadd_self inference.

…ject into fix-185920

arsenm · 2026-04-20T21:35:43Z

+    switch (II->getIntrinsicID()) {
+    case Intrinsic::fabs:
+    case Intrinsic::copysign:
+      return false;


The safer direction would be to list the cases where it is OK. You also already have the switch over all the opcodes directly in computeKnownFPClass. Can you keep this contained in computeKnownFPClass without a second set of opcode switch logic

cardigan1008 · 2026-04-29T06:36:11Z

Ping.

cardigan1008 added 2 commits March 13, 2026 12:49

[ValueTracking] Improve nofpclass inference for nsz fadd

d993500

[ValueTracking] Fix format issues

db4d943

cardigan1008 requested a review from nikic as a code owner March 13, 2026 05:00

llvmbot added llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Mar 13, 2026

nikic requested review from arsenm and dtcxzyw March 16, 2026 12:00

nikic added the floating-point Floating-point math label Mar 16, 2026

dtcxzyw reviewed Mar 16, 2026

View reviewed changes

nikic reviewed Mar 17, 2026

View reviewed changes

arsenm reviewed Mar 20, 2026

View reviewed changes

Comment thread llvm/lib/Analysis/ValueTracking.cpp Outdated

arsenm reviewed Mar 20, 2026

View reviewed changes

Comment thread llvm/lib/Analysis/ValueTracking.cpp Outdated

cardigan1008 added 2 commits April 10, 2026 16:48

[ValueTracking] Handle nsz more generically

5cf73a4

[ValueTracking] Fix format issues

f28f81f

cardigan1008 changed the title ~~[ValueTracking] Improve nofpclass inference for nsz fadd~~ [ValueTracking] Fix computeKnownFPClass handling of nsz Apr 10, 2026

cardigan1008 requested review from arsenm, dtcxzyw and nikic April 10, 2026 09:07

Merge branch 'main' into fix-185920

c1a2759

arsenm reviewed Apr 10, 2026

View reviewed changes

arsenm mentioned this pull request Apr 10, 2026

[InstSimplify][InstCombine] Handle nsz in fabs(fneg x) (llvm#151303) #187994

Closed

cardigan1008 added 2 commits April 11, 2026 18:47

[ValueTracking] Handle DAZ and exclude ops not appliable to nsz

2a2fa1c

[ValueTracking] Move check after inference and add tests

49f3e5e

cardigan1008 added 7 commits April 11, 2026 20:21

Merge branch 'fix-185920' of https://github.com/cardigan1008/llvm-pro…

48abd65

…ject into fix-185920

[ValueTracking] Polish up exlusion of applying nsz

6712cbd

[ValueTracking] Fix format issues

be3eafe

[ValueTracking] Update regressions

90fd36d

[ValueTracking] Only modify fc*Zero not subnormal

5c1e912

[ValueTracking] Update tests

fd774dd

[ValueTracking] Update tests

c0fe963

cardigan1008 requested a review from arsenm April 20, 2026 05:38

arsenm reviewed Apr 20, 2026

View reviewed changes

[ValueTracking] Whitelist for fp ops with nsz and remove dup switch

1587a81

cardigan1008 requested a review from arsenm April 22, 2026 07:15

Conversation

cardigan1008 commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 13, 2026

Uh oh!

llvmbot commented Mar 13, 2026

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

arsenm commented Mar 20, 2026

Uh oh!

Uh oh!

Uh oh!

cardigan1008 commented Apr 10, 2026

Uh oh!

cardigan1008 commented Apr 10, 2026

Uh oh!

github-actions Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🐧 Linux x64 Test Results

Uh oh!

github-actions Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🪟 Windows x64 Test Results

Uh oh!

arsenm Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

cardigan1008 Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arsenm Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

cardigan1008 commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cardigan1008 commented Mar 13, 2026 •

edited

Loading

github-actions Bot commented Apr 10, 2026 •

edited

Loading

github-actions Bot commented Apr 10, 2026 •

edited

Loading