Skip to content

Commit

Permalink
[ConstantFold] Fix misfolding fcmp of a ConstantExpr NaN with itself.
Browse files Browse the repository at this point in the history
The code incorrectly inferred that the relationship of a constant expression
to itself is FCMP_OEQ (ordered and equal), when it's actually FCMP_UEQ
(unordered *or* equal). This change corrects that, and adds some more limited
folds that can be done in this case.

Differential revision: https://reviews.llvm.org/D51216

llvm-svn: 354381
  • Loading branch information
AndrewScheidecker committed Feb 19, 2019
1 parent bddf892 commit 8ca3f38
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 7 deletions.
13 changes: 10 additions & 3 deletions llvm/lib/IR/ConstantFold.cpp
Expand Up @@ -1359,8 +1359,9 @@ static FCmpInst::Predicate evaluateFCmpRelation(Constant *V1, Constant *V2) {
assert(V1->getType() == V2->getType() &&
"Cannot compare values of different types!");

// Handle degenerate case quickly
if (V1 == V2) return FCmpInst::FCMP_OEQ;
// We do not know if a constant expression will evaluate to a number or NaN.
// Therefore, we can only say that the relation is unordered or equal.
if (V1 == V2) return FCmpInst::FCMP_UEQ;

if (!isa<ConstantExpr>(V1)) {
if (!isa<ConstantExpr>(V2)) {
Expand Down Expand Up @@ -1855,7 +1856,6 @@ Constant *llvm::ConstantFoldCompareInstruction(unsigned short pred,
default: llvm_unreachable("Unknown relation!");
case FCmpInst::FCMP_UNO:
case FCmpInst::FCMP_ORD:
case FCmpInst::FCMP_UEQ:
case FCmpInst::FCMP_UNE:
case FCmpInst::FCMP_ULT:
case FCmpInst::FCMP_UGT:
Expand Down Expand Up @@ -1901,6 +1901,13 @@ Constant *llvm::ConstantFoldCompareInstruction(unsigned short pred,
else if (pred == FCmpInst::FCMP_ONE || pred == FCmpInst::FCMP_UNE)
Result = 1;
break;
case FCmpInst::FCMP_UEQ: // We know that C1 == C2 || isUnordered(C1, C2).
// We can only partially decide this relation.
if (pred == FCmpInst::FCMP_ONE)
Result = 0;
else if (pred == FCmpInst::FCMP_UEQ)
Result = 1;
break;
}

// If we evaluated the result, return it now.
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/Transforms/ConstProp/bitcast.ll
Expand Up @@ -36,21 +36,21 @@ define i1 @bad_fcmp_constexpr_bitcast() {
ret i1 %cmp
}

; FIXME: If the bitcasts result in a NaN FP value, then "ordered and equal" would be false.
; Ensure that an "ordered and equal" fcmp of a ConstantExpr to itself is not folded, since the ConstantExpr may be a NaN.

define i1 @fcmp_constexpr_oeq(float %conv) {
; CHECK-LABEL: @fcmp_constexpr_oeq(
; CHECK-NEXT: ret i1 true
; CHECK-NEXT: ret i1 fcmp oeq (float bitcast (i32 ptrtoint (i16* @a to i32) to float), float bitcast (i32 ptrtoint (i16* @a to i32) to float))
;
%cmp = fcmp oeq float bitcast (i32 ptrtoint (i16* @a to i32) to float), bitcast (i32 ptrtoint (i16* @a to i32) to float)
ret i1 %cmp
}

; FIXME: If the bitcasts result in a NaN FP value, then "unordered or not equal" would be true.
; Ensure that an "unordered or not equal" fcmp of a ConstantExpr to itself is not folded, since the ConstantExpr may be a NaN.

define i1 @fcmp_constexpr_une(float %conv) {
; CHECK-LABEL: @fcmp_constexpr_une(
; CHECK-NEXT: ret i1 false
; CHECK-NEXT: ret i1 fcmp une (float bitcast (i32 ptrtoint (i16* @a to i32) to float), float bitcast (i32 ptrtoint (i16* @a to i32) to float))
;
%cmp = fcmp une float bitcast (i32 ptrtoint (i16* @a to i32) to float), bitcast (i32 ptrtoint (i16* @a to i32) to float)
ret i1 %cmp
Expand Down

0 comments on commit 8ca3f38

Please sign in to comment.