[ValueTracking] Fix incorrect inferrence about the signbit of sqrt

[dtcxzyw]

According to IEEE Std 754-2019, sqrt returns nan when the input is negative (except for -0). In this case, we cannot make assumptions about sign bit of the result.
BTW, alive2 seems to treat it as poison :( See AliveToolkit/alive2#1037

Fixes #92217

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: Yingwei Zheng (dtcxzyw)

Changes

According to IEEE Std 754-2019, sqrt returns nan when the input is negative (except for -0). In this case, we cannot make assumptions about sign bit of the result.
BTW, alive2 seems to treat it as poison :( See AliveToolkit/alive2#1037

Fixes #92217

---

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

3 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+1-4)
• (modified) llvm/test/Transforms/InstCombine/known-bits.ll (+15)
• (modified) llvm/unittests/Analysis/ValueTrackingTest.cpp (+1-1)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index e8c5f9b3dc25d..0f754523ba351 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -4940,11 +4940,8 @@ void computeKnownFPClass(const Value *V, const APInt &DemandedElts,
       // subnormal input could produce a negative zero output.
       const Function *F = II->getFunction();
       if (Q.IIQ.hasNoSignedZeros(II) ||
-          (F && KnownSrc.isKnownNeverLogicalNegZero(*F, II->getType()))) {
+          (F && KnownSrc.isKnownNeverLogicalNegZero(*F, II->getType())))
         Known.knownNot(fcNegZero);
-        if (KnownSrc.isKnownNeverNaN())
-          Known.signBitMustBeZero();
-      }
 
       break;
     }
diff --git a/llvm/test/Transforms/InstCombine/known-bits.ll b/llvm/test/Transforms/InstCombine/known-bits.ll
index 82cd24027e4e1..b4e79282f7012 100644
--- a/llvm/test/Transforms/InstCombine/known-bits.ll
+++ b/llvm/test/Transforms/InstCombine/known-bits.ll
@@ -1698,6 +1698,21 @@ define i32 @test_none(float nofpclass(all) %x) {
   ret i32 %and
 }
 
+; We cannot make assumptions about the sign of result of sqrt
+; when the input is a negative value (except for -0).
+define i1 @pr92217() {
+; CHECK-LABEL: @pr92217(
+; CHECK-NEXT:    [[X:%.*]] = call float @llvm.sqrt.f32(float 0xC6DEBE9E60000000)
+; CHECK-NEXT:    [[Y:%.*]] = bitcast float [[X]] to i32
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[Y]], 0
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %x = call float @llvm.sqrt.f32(float 0xC6DEBE9E60000000)
+  %y = bitcast float %x to i32
+  %cmp = icmp slt i32 %y, 0
+  ret i1 %cmp
+}
+
 define i8 @test_icmp_add(i8 %n, i8 %n2, i8 %other) {
 ; CHECK-LABEL: @test_icmp_add(
 ; CHECK-NEXT:  entry:
diff --git a/llvm/unittests/Analysis/ValueTrackingTest.cpp b/llvm/unittests/Analysis/ValueTrackingTest.cpp
index 8738af91b652b..a30db468c7729 100644
--- a/llvm/unittests/Analysis/ValueTrackingTest.cpp
+++ b/llvm/unittests/Analysis/ValueTrackingTest.cpp
@@ -2005,7 +2005,7 @@ TEST_F(ComputeKnownFPClassTest, SqrtNszSignBit) {
         computeKnownFPClass(A4, M->getDataLayout(), fcAllFlags, 0, nullptr,
                             nullptr, nullptr, nullptr, /*UseInstrInfo=*/true);
     EXPECT_EQ(fcPositive | fcQNan, UseInstrInfoNSZNoNan.KnownFPClasses);
-    EXPECT_EQ(false, UseInstrInfoNSZNoNan.SignBit);
+    EXPECT_EQ(std::nullopt, UseInstrInfoNSZNoNan.SignBit);
 
     KnownFPClass NoUseInstrInfoNSZNoNan =
         computeKnownFPClass(A4, M->getDataLayout(), fcAllFlags, 0, nullptr,

[arsenm]

&& if the source is known positive (or the op has the nnan flag)

[dtcxzyw]

Isn't it covered by the above code?

llvm-project/llvm/lib/Analysis/ValueTracking.cpp

Lines 4932 to 4937 in b27eb0a

	// Any negative value besides -0 returns a nan.
	if (KnownSrc.isKnownNeverNaN() && KnownSrc.cannotBeOrderedLessThanZero())
	Known.knownNot(fcNan);
	// The only negative value that can be returned is -0 for -0 inputs.
	Known.knownNot(fcNegInf | fcNegSubnormal | fcNegNormal);

[arsenm]

Semi-related, this should be using propagateNaN instead of repeating the snan logic

[arsenm]

I think we are missing handling of the nnan flag on the sqrt instruction itself

[arsenm]

Or that just comes from the RAII thing

[dtcxzyw]

I think we are missing handling of the nnan flag on the sqrt instruction itself

See

llvm-project/llvm/include/llvm/Analysis/ValueTracking.h

Lines 510 to 527 in f5c8242

	/// Wrapper to account for known fast math flags at the use instruction.
	inline KnownFPClass computeKnownFPClass(const Value *V, FastMathFlags FMF,
	FPClassTest InterestedClasses,
	unsigned Depth,
	const SimplifyQuery &SQ) {
	if (FMF.noNaNs())
	InterestedClasses &= ~fcNan;
	if (FMF.noInfs())
	InterestedClasses &= ~fcInf;
	KnownFPClass Result = computeKnownFPClass(V, InterestedClasses, Depth, SQ);
	if (FMF.noNaNs())
	Result.KnownFPClasses &= ~fcNan;
	if (FMF.noInfs())
	Result.KnownFPClasses &= ~fcInf;
	return Result;
	}

[dtcxzyw]

See the test sqrt_negative_input_nnan.

[dtcxzyw]

/cherry-pick 04ae6e6

[llvmbot]

Failed to cherry-pick: 04ae6e6

https://github.com/llvm/llvm-project/actions/runs/9159786286

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

[nikic]

@dtcxzyw LLVM 18 does not accept backports anymore.