This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Mark slow tests as unsupported on GCC
ClosedPublic

Authored by ldionne on Jun 12 2023, 11:27 AM.

Details

Reviewers
Mordante
philnik
Group Reviewers
Restricted Project
Commits
rG520c7fbbd035: [libc++] Mark slow tests as unsupported on GCC
Summary

Some tests in our test suite are unbelievably slow on GCC due to the
use of the always_inline attribute. See [1] for more details.

This patch introduces the GCC-ALWAYS_INLINE-FIXME lit feature to
disable tests that are plagued by that issue. At the same time, it
moves several existing tests from ad-hoc UNSUPPORTED: gcc-12 markup
to the new GCC-ALWAYS_INLINE-FIXME feature, and marks the slowest tests
reported by the CI as UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME.

[1]: https://discourse.llvm.org/t/rfc-stop-supporting-extern-instantiations-with-gcc/71277/1

Diff Detail

Event Timeline

ldionne created this revision.Jun 12 2023, 11:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2023, 11:27 AM
ldionne requested review of this revision.Jun 12 2023, 11:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2023, 11:27 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante accepted this revision as: Mordante.Jun 12 2023, 11:51 AM
Mordante added subscribers: philnik, Mordante.

Once the CI finishes, can you update the difference in runtime for our CI.
I know the format part does nothing, but the ranges part would be interesting to know.

I will look at discourse later. Still I'm happy with the patch, since it offers a better message for format/chrono.

LGTM! Since @philnik wrote on Discourse I leave the final approval to him.

libcxx/test/std/containers/container.adaptors/container.adaptors.format/format.functions.format.pass.cpp
11

Am I right to assume you changed all these cases in format/chrono? If that is the case I will abandon D135787.

philnik accepted this revision.Jun 12 2023, 12:08 PM
This revision is now accepted and ready to land.Jun 12 2023, 12:08 PM
ldionne marked an inline comment as done.Jun 13 2023, 10:14 AM

Once the CI finishes, can you update the difference in runtime for our CI.
I know the format part does nothing, but the ranges part would be interesting to know.

I will look at discourse later. Still I'm happy with the patch, since it offers a better message for format/chrono.

LGTM! Since @philnik wrote on Discourse I leave the final approval to him.

I just checked and with this patch applied, the GCC run took ~15 minutes as opposed to ~24 minutes lately.

libcxx/test/std/containers/container.adaptors/container.adaptors.format/format.functions.format.pass.cpp
11

I think I caught everything, yes.

ldionne updated this revision to Diff 530968.Jun 13 2023, 10:20 AM
ldionne marked an inline comment as done.

Add markup for all tests that take more than 100 seconds.

This revision was landed with ongoing or failed builds.Jun 13 2023, 10:21 AM
This revision was automatically updated to reflect the committed changes.

I just checked and with this patch applied, the GCC run took ~15 minutes as opposed to ~24 minutes lately.

That's a nice improvement, thanks!