Skip to content

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Oct 8, 2024

Note: The current implementation doesn't return optimal result for fcmp one/une x, +/-inf since we don't handle this case in #110891. Maybe we can make it optimal after seeing some real-world cases.

@dtcxzyw dtcxzyw requested a review from arsenm October 8, 2024 07:03
@llvmbot llvmbot added the llvm:ir label Oct 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 8, 2024

@llvm/pr-subscribers-llvm-ir

Author: Yingwei Zheng (dtcxzyw)

Changes

Note: The current implementation doesn't return optimal result for fcmp one/une x, +/-inf since we don't handle this case in #110891. Maybe we can make it optimal after seeing some real-world cases.


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

2 Files Affected:

  • (modified) llvm/lib/IR/ConstantFPRange.cpp (+4-1)
  • (modified) llvm/unittests/IR/ConstantFPRangeTest.cpp (+24)
diff --git a/llvm/lib/IR/ConstantFPRange.cpp b/llvm/lib/IR/ConstantFPRange.cpp
index d3c89daa9ce148..75091881285248 100644
--- a/llvm/lib/IR/ConstantFPRange.cpp
+++ b/llvm/lib/IR/ConstantFPRange.cpp
@@ -268,7 +268,10 @@ ConstantFPRange::makeSatisfyingFCmpRegion(FCmpInst::Predicate Pred,
 std::optional<ConstantFPRange>
 ConstantFPRange::makeExactFCmpRegion(FCmpInst::Predicate Pred,
                                      const APFloat &Other) {
-  return std::nullopt;
+  if ((Pred == FCmpInst::FCMP_UNE || Pred == FCmpInst::FCMP_ONE) &&
+      !Other.isNaN())
+    return std::nullopt;
+  return makeSatisfyingFCmpRegion(Pred, ConstantFPRange(Other));
 }
 
 bool ConstantFPRange::fcmp(FCmpInst::Predicate Pred,
diff --git a/llvm/unittests/IR/ConstantFPRangeTest.cpp b/llvm/unittests/IR/ConstantFPRangeTest.cpp
index 3ce64c64447e21..255f62d77b748d 100644
--- a/llvm/unittests/IR/ConstantFPRangeTest.cpp
+++ b/llvm/unittests/IR/ConstantFPRangeTest.cpp
@@ -743,4 +743,28 @@ TEST_F(ConstantFPRangeTest, fcmp) {
   }
 }
 
+TEST_F(ConstantFPRangeTest, makeExactFCmpRegion) {
+  for (auto Pred : FCmpInst::predicates()) {
+    EnumerateValuesInConstantFPRange(
+        ConstantFPRange::getFull(APFloat::Float8E4M3()),
+        [Pred](const APFloat &V) {
+          std::optional<ConstantFPRange> Res =
+              ConstantFPRange::makeExactFCmpRegion(Pred, V);
+          ConstantFPRange Allowed =
+              ConstantFPRange::makeAllowedFCmpRegion(Pred, ConstantFPRange(V));
+          ConstantFPRange Satisfying =
+              ConstantFPRange::makeSatisfyingFCmpRegion(Pred,
+                                                        ConstantFPRange(V));
+          if (Allowed == Satisfying)
+            EXPECT_EQ(Res, Allowed) << "Wrong result for makeExactFCmpRegion("
+                                    << Pred << ", " << V << ").";
+          else
+            EXPECT_FALSE(Res.has_value())
+                << "Wrong result for makeExactFCmpRegion(" << Pred << ", " << V
+                << ").";
+        },
+        /*IgnoreNaNPayload=*/true);
+  }
+}
+
 } // anonymous namespace

}
}

TEST_F(ConstantFPRangeTest, makeExactFCmpRegion) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test only takes about 1ms with optimized build.

@arsenm arsenm added the floating-point Floating-point math label Oct 8, 2024
@dtcxzyw dtcxzyw merged commit a3a253d into llvm:main Oct 8, 2024
10 of 12 checks passed
@dtcxzyw dtcxzyw deleted the cfr-exact-fcmp branch October 8, 2024 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
floating-point Floating-point math llvm:ir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants