[SYCL][E2E] Print features detected by lit #16866
Conversation
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
aelovikov-intel
force-pushed
the
print-features
branch
from
c9c53d8 to
3151a52
Compare
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
This is a bit controversial as the output is noisy. Feel free to vote via thumb up/down. |
aelovikov-intel
requested review from
AlexeySachkov,
sarnex,
maarquitos14,
uditagarwal97,
sergey-semenov,
steffenlarsen and
JackAKirk
sycl/test-e2e/lit.cfg.py
Outdated
| @@ -890,6 +890,13 @@ def open_check_file(file_name): | |||
| ("%clang", " " + config.dpcpp_compiler + " " + config.c_flags) | |||
| ) | |||
|
|
|||
| lit_config.note( | |||
There was a problem hiding this comment.
How about we print all features only if --verbose / -v is specified?
There was a problem hiding this comment.
I don't think it would be useful that much. This PR only makes sense if we enable it by default in all our CI jobs, meaning the noise will be there.
There was a problem hiding this comment.
I don't think it would be useful that much. This PR only makes sense if we enable it by default in all our CI jobs, meaning the noise will be there.
IIUC, we already pass -v to llvm-lit in our CI jobs: https://github.com/intel/llvm/blob/sycl/devops/actions/run-tests/e2e/action.yml#L62.
I don't mind printing features in CI jobs as they would be useful for debugging (it's not that noisy :)). My preference would be to not print features unnecessarily when we invoke llvm-lit locally. We already print device aspects unconditionally and llvm-lit's output (even without --verbose) can be big when we have multiple devices in the environment.
|
I think this is a good change. I do understand the concern about noisy output, but the information (in particular the common aspects) is generally hard to otherwise gather and has an important impact on how the testing is done, so the noise at the beginning of the already fairly verbose LIT output seems worth it to me. |
|
Seems fine to me too, the location of the extra output is localized. |
aelovikov-intel
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
aelovikov-intel
force-pushed
the
print-features
branch
from
4b6317d to
9a1bccf
Compare
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
No description provided.