Skip to content

[SYCL][E2E] Add some compile-time "tracking" tests #16648

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 1 commit into from
Jan 16, 2025

Conversation

aelovikov-intel
Copy link
Contributor

We run test-e2e/PerformanceTests in post-commit and that can be used for

  • tracking the compilation time on these tests over time
  • running them in pre-commit to measure expected improvements

I also think that since PerformanceTests are "opt-in" in precommit, there is little sense in enforcing no <sycl/sycl.hpp> header in them.

We run `test-e2e/PerformanceTests` in post-commit and that can be used
for

* tracking the compilation time on these tests over time
* running them in pre-commit to measure expected improvements

I also think that since PerformanceTests are "opt-in" in precommit,
there is little sense in enforcing no `<sycl/sycl.hpp>` header in them.
@aelovikov-intel aelovikov-intel added the run-perf-tests Run performance tests in pre-commit (normally part of post-commit only) label Jan 15, 2025
@aelovikov-intel aelovikov-intel requested a review from a team as a code owner January 15, 2025 18:07
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jan 15, 2025
aelovikov-intel added a commit that referenced this pull request Jan 15, 2025
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me!

As future work, would it make sense to unify core_hpp.cpp and sycl_hpp.cpp into a tool that tries compiling each header file individually? If we start tracking it, it might help us narrow down sources of regressions.

@aelovikov-intel aelovikov-intel merged commit f0d0f7c into intel:sycl Jan 16, 2025
21 of 22 checks passed
@aelovikov-intel aelovikov-intel deleted the compile-time-tests branch January 16, 2025 16:26
@aelovikov-intel
Copy link
Contributor Author

Seems reasonable to me!

As future work, would it make sense to unify core_hpp.cpp and sycl_hpp.cpp into a tool that tries compiling each header file individually? If we start tracking it, it might help us narrow down sources of regressions.

I have some doubts about that. With how our headers interdependent, that would result in lots of regressions reported in multiple headers at once, not helping to narrow it down to the one causing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-perf-tests Run performance tests in pre-commit (normally part of post-commit only)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants