Skip to content

[ConstantFPRange][UnitTests] Guard exhaustive checks by EXPENSIVE_CHECKS #111276

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

Merged
merged 2 commits into from
Oct 8, 2024

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Oct 6, 2024

Addresses comment #111056 (comment).

@dtcxzyw dtcxzyw requested review from arsenm, vporpo and aeubanks October 6, 2024 03:39
@llvmbot llvmbot added the llvm:ir label Oct 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2024

@llvm/pr-subscribers-llvm-ir

Author: Yingwei Zheng (dtcxzyw)

Changes

Exhaustive testing is expensive in debug builds, so we only do it when optimizations are enabled. We don't use EXPENSIVE_CHECKS here because it's not enabled by default in optimized builds.

Addresses comment #111056 (comment).


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

1 Files Affected:

  • (modified) llvm/unittests/IR/ConstantFPRangeTest.cpp (+29)
diff --git a/llvm/unittests/IR/ConstantFPRangeTest.cpp b/llvm/unittests/IR/ConstantFPRangeTest.cpp
index 158d08f9b77a0a..faeb04a83bb927 100644
--- a/llvm/unittests/IR/ConstantFPRangeTest.cpp
+++ b/llvm/unittests/IR/ConstantFPRangeTest.cpp
@@ -15,6 +15,31 @@ using namespace llvm;
 
 namespace {
 
+// Exhaustive testing is expensive in debug builds, so we only do it when
+// optimizations are enabled. We don't use EXPENSIVE_CHECKS here because it's
+// not enabled by default in optimized builds.
+#if defined(__GNUC__)
+// GCC and GCC-compatible compilers define __OPTIMIZE__ when optimizations are
+// enabled.
+#if defined(__OPTIMIZE__)
+#define LLVM_ENABLE_CFR_EXPENSIVE_CHECKS 1
+#else
+#define LLVM_ENABLE_CFR_EXPENSIVE_CHECKS 0
+#endif
+#elif defined(_MSC_VER)
+// MSVC doesn't have a predefined macro indicating if optimizations are enabled.
+// Use _DEBUG instead. This macro actually corresponds to the choice between
+// debug and release CRTs, but it is a reasonable proxy.
+#if defined(_DEBUG)
+#define LLVM_ENABLE_CFR_EXPENSIVE_CHECKS 0
+#else
+#define LLVM_ENABLE_CFR_EXPENSIVE_CHECKS 1
+#endif
+#else
+// Otherwise, for an unknown compiler, assume this is an optimized build.
+#define LLVM_ENABLE_CFR_EXPENSIVE_CHECKS 1
+#endif
+
 class ConstantFPRangeTest : public ::testing::Test {
 protected:
   static const fltSemantics &Sem;
@@ -435,6 +460,7 @@ TEST_F(ConstantFPRangeTest, FPClassify) {
   EXPECT_EQ(SomePos.getSignBit(), false);
   EXPECT_EQ(SomeNeg.getSignBit(), true);
 
+#if LLVM_ENABLE_CFR_EXPENSIVE_CHECKS
   EnumerateConstantFPRanges(
       [](const ConstantFPRange &CR) {
         unsigned Mask = fcNone;
@@ -458,6 +484,7 @@ TEST_F(ConstantFPRangeTest, FPClassify) {
         EXPECT_EQ(Mask, CR.classify()) << CR;
       },
       /*Exhaustive=*/true);
+#endif
 }
 
 TEST_F(ConstantFPRangeTest, Print) {
@@ -500,6 +527,7 @@ TEST_F(ConstantFPRangeTest, MismatchedSemantics) {
 #endif
 
 TEST_F(ConstantFPRangeTest, makeAllowedFCmpRegion) {
+#if LLVM_ENABLE_CFR_EXPENSIVE_CHECKS
   for (auto Pred : FCmpInst::predicates()) {
     EnumerateConstantFPRanges(
         [Pred](const ConstantFPRange &CR) {
@@ -529,6 +557,7 @@ TEST_F(ConstantFPRangeTest, makeAllowedFCmpRegion) {
         },
         /*Exhaustive=*/false);
   }
+#endif
 }
 
 } // anonymous namespace

@aeubanks
Copy link
Contributor

aeubanks commented Oct 7, 2024

I ran IRTests and makeAllowedFCmpRegion is the longest test at 7183 ms, second is MismatchedSemantics at 1131 ms, then after that is some random death test at 472 ms.

7s in an optimized build is still non-trivial. is it possible to do some basic checks in a normal build, and exhaustive in an expensive checks build?

@dtcxzyw dtcxzyw changed the title [ConstantFPRange][UnitTests] Disable exhaustive testing in debug builds [ConstantFPRange][UnitTests] Guard exhaustive checks by EXPENSIVE_CHECKS Oct 8, 2024
Copy link
Contributor

@aeubanks aeubanks left a comment

Choose a reason for hiding this comment

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

thanks!

@dtcxzyw dtcxzyw merged commit db4874c into llvm:main Oct 8, 2024
7 of 9 checks passed
@dtcxzyw dtcxzyw deleted the cfr-debug branch October 8, 2024 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants