-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[ValueTracking] Fix assertion failure when computeKnownFPClass
returns fcNone
#92355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-analysis Author: Yingwei Zheng (dtcxzyw) ChangesFixes #92084 (comment). Full diff: https://github.com/llvm/llvm-project/pull/92355.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index c8c527a2d4d2f..087cfcc792ee4 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1126,7 +1126,8 @@ static void computeKnownBitsFromOperator(const Operator *I,
KnownFPClass Result = computeKnownFPClass(V, fcAllFlags, Depth + 1, Q);
FPClassTest FPClasses = Result.KnownFPClasses;
- if (Result.isKnownNever(fcNormal | fcSubnormal | fcNan)) {
+ if (FPClasses != fcNone &&
+ Result.isKnownNever(fcNormal | fcSubnormal | fcNan)) {
Known.Zero.setAllBits();
Known.One.setAllBits();
diff --git a/llvm/test/Transforms/InstCombine/known-bits.ll b/llvm/test/Transforms/InstCombine/known-bits.ll
index 7a38020500517..cb404c0b7f889 100644
--- a/llvm/test/Transforms/InstCombine/known-bits.ll
+++ b/llvm/test/Transforms/InstCombine/known-bits.ll
@@ -1658,5 +1658,45 @@ define i32 @test_qnan_only(float nofpclass(snan sub norm zero inf) %x) {
ret i32 %and
}
+; Make sure that we don't crash when the use of x is unreachable.
+define i64 @pr92084(double %x) {
+; CHECK-LABEL: @pr92084(
+; CHECK-NEXT: [[CMP:%.*]] = fcmp uno double [[X:%.*]], 0.000000e+00
+; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN1:%.*]], label [[IF_ELSE:%.*]]
+; CHECK: if.then1:
+; CHECK-NEXT: br i1 [[CMP]], label [[IF_ELSE]], label [[IF_THEN2:%.*]]
+; CHECK: if.then2:
+; CHECK-NEXT: [[CAST:%.*]] = bitcast double [[X]] to i64
+; CHECK-NEXT: [[AND:%.*]] = and i64 [[CAST]], 1
+; CHECK-NEXT: ret i64 [[AND]]
+; CHECK: if.else:
+; CHECK-NEXT: ret i64 0
+;
+ %cmp = fcmp uno double %x, 0.000000e+00
+ br i1 %cmp, label %if.then1, label %if.else
+
+if.then1:
+ br i1 %cmp, label %if.else, label %if.then2
+
+if.then2:
+ %cast = bitcast double %x to i64
+ %and = and i64 %cast, 1
+ ret i64 %and
+
+if.else:
+ ret i64 0
+}
+
+define i32 @test_none(float nofpclass(all) %x) {
+; CHECK-LABEL: @test_none(
+; CHECK-NEXT: [[Y:%.*]] = bitcast float [[X:%.*]] to i32
+; CHECK-NEXT: [[AND:%.*]] = and i32 [[Y]], 4194304
+; CHECK-NEXT: ret i32 [[AND]]
+;
+ %y = bitcast float %x to i32
+ %and = and i32 %y, 4194304
+ ret i32 %and
+}
+
declare void @use(i1)
declare void @sink(i8)
|
llvm/lib/Analysis/ValueTracking.cpp
Outdated
// Treat it as zero if the use of I is unreachable. | ||
if (FPClasses == fcNone) { | ||
Known.setAllZero(); | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should do this as a separate PR, this probably has the need-to-consistently-be-zero-for-undef problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will leave it as a todo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably OK as long as we handle undef correctly inside computeKnownFPClass, the contradictory paths implies UB happened somewhere
Fixes #92084 (comment).