Skip to content

Do not run "check-runtimes" if "check-all" will be also be run #472

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DavidSpickett
Copy link
Contributor

For a long time, at least since llvm/llvm-project@f39a971, the runtimes tests have been part of check-all.

I only noticed this the other day when our bot failed the same test twice.

https://lab.llvm.org/buildbot/#/builders/161/builds/6483

The same thing happened on some ppc bots:
https://lab.llvm.org/buildbot/#/builders/145/builds/7641 https://lab.llvm.org/buildbot/#/builders/95/builds/14342

This is because they run the targets separately:
ninja check-all
ninja check-runtimes

So I have removed the check-runtimes target from these.

clang-ppc64le-linux-multistage didn't run the test twice: https://lab.llvm.org/buildbot/#/builders/76/builds/10353

(twice per stage that is)

Because it runs the tests as: ninja check-all check-runtimes

And ninja knows that one is part of the other.

I've removed check-runtimes from that bot anyway, just for clarity. The tests will still be run as part of check-all.

For a long time, at least since llvm/llvm-project@f39a971,
the runtimes tests have been part of check-all.

I only noticed this the other day when our bot failed the
same test twice.

https://lab.llvm.org/buildbot/#/builders/161/builds/6483

The same thing happened on some ppc bots:
https://lab.llvm.org/buildbot/#/builders/145/builds/7641
https://lab.llvm.org/buildbot/#/builders/95/builds/14342

This is because they run the targets separately:
ninja check-all
ninja check-runtimes

So I have removed the check-runtimes target from these.

clang-ppc64le-linux-multistage didn't run the test twice:
https://lab.llvm.org/buildbot/#/builders/76/builds/10353

(twice per stage that is)

Because it runs the tests as: ninja check-all check-runtimes

And ninja knows that one is part of the other.

I've removed check-runtimes from that bot anyway, just for clarity.
The tests will still be run as part of check-all.
@DavidSpickett DavidSpickett requested a review from omjavaid June 11, 2025 10:24
@DavidSpickett
Copy link
Contributor Author

DavidSpickett commented Jun 11, 2025

Emailed the PowerPC folks to get their feedback.

Copy link
Contributor

@omjavaid omjavaid left a comment

Choose a reason for hiding this comment

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

LGTM

@omjavaid
Copy link
Contributor

On a side note can you confirm if we are running check flang-rt as part of check-all for flang bots.

@DavidSpickett
Copy link
Contributor Author

We are. clang-aarch64-sve-vla for example:

$ grep "flang-rt" stdio\ \(18\) | head
[1/8] Generating Fortran dyndep file flang-rt/lib/runtime/CMakeFiles/flang_rt.runtime.static.dir/Fortran.dd
PASS: flang-rt :: Driver/ctofortran.f90 (8234 of 98392)
PASS: flang-rt-Unit :: Runtime/./RuntimeTests/85/122 (14256 of 98392)
PASS: flang-rt-Unit :: Runtime/./RuntimeTests/86/122 (15218 of 98392)
PASS: flang-rt-Unit :: Runtime/./RuntimeTests/82/122 (16299 of 98392)
PASS: flang-rt :: Runtime/no-cpp-dep.c (20681 of 98392)
PASS: flang-rt-Unit :: Runtime/./RuntimeTests/96/122 (24734 of 98392)
PASS: flang-rt-Unit :: Runtime/./RuntimeTests/2/122 (25035 of 98392)
PASS: flang-rt-Unit :: Runtime/./RuntimeTests/4/122 (25328 of 98392)
PASS: flang-rt-Unit :: Runtime/./RuntimeTests/15/122 (26418 of 98392)

@daltenty daltenty requested a review from kamaub June 13, 2025 14:01
@daltenty
Copy link
Member

IIRC there used to be a historical bug that this worked around, where some of the compiler-rt tests weren't run under check-all, but I don't think that is even working anymore as I don't see the check-builtins tests being run in either check-all or check-runtimes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants