Skip to content

[test] Disable makeAllowedFCmpRegion due to taking too long #111056

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

Closed
wants to merge 1 commit into from

Conversation

aeubanks
Copy link
Contributor

@aeubanks aeubanks commented Oct 3, 2024

This was added in #110082. It takes 32s in an optimized build of LLVM, and doesn't finish after multiple minutes in an -O0 build. It's iterating all possible fp values 3 levels deep. Disable until the test is fixed to not take so much time.

This was added in llvm#110082. It takes 32s in an optimized build of LLVM,
and doesn't finish after multiple minutes in an -O0 build. It's
iterating all possible fp values 3 levels deep. Disable until the
test is fixed to not take so much time.
@aeubanks aeubanks requested review from arsenm, vporpo and dtcxzyw October 3, 2024 20:20
@llvmbot llvmbot added the llvm:ir label Oct 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2024

@llvm/pr-subscribers-llvm-ir

Author: Arthur Eubanks (aeubanks)

Changes

This was added in #110082. It takes 32s in an optimized build of LLVM, and doesn't finish after multiple minutes in an -O0 build. It's iterating all possible fp values 3 levels deep. Disable until the test is fixed to not take so much time.


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

1 Files Affected:

  • (modified) llvm/unittests/IR/ConstantFPRangeTest.cpp (+3)
diff --git a/llvm/unittests/IR/ConstantFPRangeTest.cpp b/llvm/unittests/IR/ConstantFPRangeTest.cpp
index 17a08207fe1ba0..ae0ea8069fff5f 100644
--- a/llvm/unittests/IR/ConstantFPRangeTest.cpp
+++ b/llvm/unittests/IR/ConstantFPRangeTest.cpp
@@ -443,6 +443,9 @@ TEST_F(ConstantFPRangeTest, MismatchedSemantics) {
 #endif
 
 TEST_F(ConstantFPRangeTest, makeAllowedFCmpRegion) {
+  // TODO: reenable once test is fixed to run more quickly.
+  GTEST_SKIP();
+
   for (auto Pred : FCmpInst::predicates()) {
     EnumerateConstantFPRanges(
         [Pred](const ConstantFPRange &CR) {

Copy link
Contributor

@vporpo vporpo left a comment

Choose a reason for hiding this comment

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

I was thinking of guarding this under NDEBUG, but 32s in Release sounds too much.

@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 4, 2024

I have sped up the test run from 30.22s -> 10.59s by ignoring the payload in NaN values. Is it acceptable?

@vporpo
Copy link
Contributor

vporpo commented Oct 4, 2024

I have sped up the test run from 30.22s -> 10.59s by ignoring the payload in NaN values. Is it acceptable?

What about the debug build? If it is 3 times faster, it would still be more than 2 and a half minutes, which is still too long.

@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 4, 2024

I have sped up the test run from 30.22s -> 10.59s by ignoring the payload in NaN values. Is it acceptable?

What about the debug build? If it is 3 times faster, it would still be more than 2 and a half minutes, which is still too long.

I don't have enough space to test the debug build. But even if I disable this test, it still takes 10.58s to run the other existing unit tests.

@vporpo
Copy link
Contributor

vporpo commented Oct 4, 2024

I don't have enough space to test the debug build.

I tried it and it takes 64 seconds in Debug. The next slowest one is ConstantFPRangeTest.FPClassify at 13s.

@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 4, 2024

The fix has been landed in 856774d.

@dtcxzyw dtcxzyw closed this Oct 4, 2024
@vporpo
Copy link
Contributor

vporpo commented Oct 4, 2024

IRTests used to take 57s for 782 tests and it now takes 2m2s for 783 tests. This is more than 100% increase just from one test, so it is still an issue for the Debug build.

@vporpo vporpo reopened this Oct 4, 2024
@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 4, 2024

IRTests used to take 57s for 782 tests and it now takes 2m2s for 783 tests. This is more than 100% increase just from one test, so it is still an issue for the Debug build.

I am sorry I cannot make it faster on debug build.
@aeubanks Can you make these exhaustive tests guarded by #ifndef NDEBUG?

@arsenm
Copy link
Contributor

arsenm commented Oct 4, 2024

I am sorry I cannot make it faster on debug build. @aeubanks Can you make these exhaustive tests guarded by #ifndef NDEBUG?

NDEBUG won't catch debug build, and this should still run in rel + asserts. EXPENSIVE_CHECKS maybe?

Maybe there is something else to speed it up?

@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 4, 2024

Maybe there is something else to speed it up?

With debug build:

[----------] 12 tests from ConstantFPRangeTest
[ RUN      ] ConstantFPRangeTest.Basics
[       OK ] ConstantFPRangeTest.Basics (0 ms)
[ RUN      ] ConstantFPRangeTest.Equality
[       OK ] ConstantFPRangeTest.Equality (0 ms)
[ RUN      ] ConstantFPRangeTest.SingleElement
[       OK ] ConstantFPRangeTest.SingleElement (0 ms)
[ RUN      ] ConstantFPRangeTest.ExhaustivelyEnumerate
[       OK ] ConstantFPRangeTest.ExhaustivelyEnumerate (70 ms)
[ RUN      ] ConstantFPRangeTest.Enumerate
[       OK ] ConstantFPRangeTest.Enumerate (1 ms)
[ RUN      ] ConstantFPRangeTest.IntersectWith
[       OK ] ConstantFPRangeTest.IntersectWith (0 ms)
[ RUN      ] ConstantFPRangeTest.UnionWith
[       OK ] ConstantFPRangeTest.UnionWith (0 ms)
[ RUN      ] ConstantFPRangeTest.FPClassify
[       OK ] ConstantFPRangeTest.FPClassify (2719 ms)
[ RUN      ] ConstantFPRangeTest.Print
[       OK ] ConstantFPRangeTest.Print (0 ms)
[ RUN      ] ConstantFPRangeTest.NonCanonicalEmptySet
[       OK ] ConstantFPRangeTest.NonCanonicalEmptySet (246 ms)
[ RUN      ] ConstantFPRangeTest.MismatchedSemantics
[       OK ] ConstantFPRangeTest.MismatchedSemantics (1221 ms)
[ RUN      ] ConstantFPRangeTest.makeAllowedFCmpRegion
[       OK ] ConstantFPRangeTest.makeAllowedFCmpRegion (52500 ms)
[----------] 12 tests from ConstantFPRangeTest (56758 ms total)
...
[==========] 783 tests from 109 test suites ran. (107440 ms total)
[  PASSED  ] 783 tests.

Please give me one more day to do some profiling.

The next slowest one is ConstantFPRangeTest.FPClassify at 13s.

Setting Exhaustive=true will reduce the test time of ConstantFPRangeTest.FPClassify from 2719ms to 61ms.

@dtcxzyw
Copy link
Member

dtcxzyw commented Oct 5, 2024

|--90.39%--(anonymous namespace)::ConstantFPRangeTest_makeAllowedFCmpRegion_Test::TestBody()::{lambda(llvm::ConstantFPRange const&)#1}::operator()(llvm::ConstantFPRange const&) const::{lambda(llvm::APFloat const&)#1}::operator()
                          |                                |          |          
                          |                                |          |--74.99%--(anonymous namespace)::AnyOfValueInConstantFPRange<(anonymous namespace)::ConstantFPRangeTest_makeAllowedFCmpRegion_Test::TestBody()::{lambda(llvm::ConstantFPRange const&)#1}::operator()(llvm::ConstantFPRange const&) const::{lambda(llvm::APFloat const&)#1}::operator()(llvm::APFloat const) const::{lambda(llvm::APFloat const)#1}>
                          |                                |          |          |          
                          |                                |          |          |--56.08%--bool (anonymous namespace)::AnyOfValueInConstantFPRange<(anonymous namespace)::ConstantFPRangeTest_makeAllowedFCmpRegion_Test::TestBody()::{lambda(llvm::ConstantFPRange const&)#1}::operator()(llvm::ConstantFPRange const&) const::{lambda(llvm::APFloat const&)#1}::operator()(llvm::APFloat const) const::{lambda(llvm::APFloat const)#1}>(llvm::ConstantFPRange const&, (anonymous namespace)::ConstantFPRangeTest_makeAllowedFCmpRegion_Test::TestBody()::{lambda(llvm::ConstantFPRange const&)#1}::operator()(llvm::ConstantFPRange const&) const::{lambda(llvm::APFloat const&)#1}::operator()(llvm::APFloat const) const::{lambda(llvm::APFloat const)#1}, bool)::{lambda({lambda(llvm::ConstantFPRange const&)#1}&)#1}::operator()
                          |                                |          |          |          |          
                          |                                |          |          |          |--46.40%--(anonymous namespace)::strictNext
                          |                                |          |          |          |          |          
                          |                                |          |          |          |          |--40.51%--llvm::APFloat::next

I cannot make it faster. I tried to cache the result of strictNext. But it took ~50% more time than before :(

EXPENSIVE_CHECKS maybe?

How about using __OPTIMIZE__/_DEBUG as we does for LLVM_IS_DEBUG_BUILD?

#if defined(__GNUC__)
// GCC and GCC-compatible compilers define __OPTIMIZE__ when optimizations are
// enabled.
# if defined(__OPTIMIZE__)
# define LLVM_IS_DEBUG_BUILD 0
# else
# define LLVM_IS_DEBUG_BUILD 1
# 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_IS_DEBUG_BUILD 1
# else
# define LLVM_IS_DEBUG_BUILD 0
# endif
#else
// Otherwise, for an unknown compiler, assume this is an optimized build.
# define LLVM_IS_DEBUG_BUILD 0
#endif
namespace {
class VersionPrinter {
public:
void print(std::vector<VersionPrinterTy> ExtraPrinters = {}) {
raw_ostream &OS = outs();
#ifdef PACKAGE_VENDOR
OS << PACKAGE_VENDOR << " ";
#else
OS << "LLVM (http://llvm.org/):\n ";
#endif
OS << PACKAGE_NAME << " version " << PACKAGE_VERSION << "\n ";
#if LLVM_IS_DEBUG_BUILD
OS << "DEBUG build";
#else
OS << "Optimized build";
#endif
#ifndef NDEBUG
OS << " with assertions";
#endif
OS << ".\n";

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.

5 participants