[SYCL][E2E] Use configure_lit_site_cfg in in-tree builds #8854

frasercrmck · 2023-03-29T09:38:45Z

This provides the added benefit that it will automatically map the config paths from the build directories to the corresponding source ones in llvm-lit. With this, one should be able to run lit tests directly from source paths.

Note also that this changes the configured cfg file's name from lit.site.cfg to lit.site.cfg.py, but this aligns us with the predominant style across the llvm project.

frasercrmck · 2023-04-10T09:31:24Z

ping, maybe @aelovikov-intel?

aelovikov-intel · 2023-04-10T15:01:08Z

Thanks for pinging me, this one is great! I've been wondering if that can be achieved but didn't have time post test-suite move to look into it myself.

That said, it only works for the non-standalone configuration (when we configure both the compiler/RT and the E2E tests in a single invocation). The standalone configuration used in the CI (both here and a different flavor internally) breaks as can be seen by the failing pre-commit checks in this PR.

I think we need to guard the fix with

if(SYCL_TEST_E2E_STANDALONE)

and use the old code for the else path.

aelovikov-intel · 2023-04-10T15:52:58Z

Also, it might make sense to modify sycl/test-e2e/README.md#build-and-run-tests to describe that benefit when using single cmake configuration.

frasercrmck · 2023-04-11T09:32:59Z

That said, it only works for the non-standalone configuration (when we configure both the compiler/RT and the E2E tests in a single invocation). The standalone configuration used in the CI (both here and a different flavor internally) breaks as can be seen by the failing pre-commit checks in this PR.

I think we need to guard the fix with
if(SYCL_TEST_E2E_STANDALONE)
and use the old code for the else path.

Good spot, I had missed that in the CI output. Thank you!

Also, it might make sense to modify sycl/test-e2e/README.md#build-and-run-tests to describe that benefit when using single cmake configuration.

Good idea; done. I was a bit confused by one of the existing lines in the README about whether it was referring to a standalone or non-standalone build. I think it was a mistake in the old readme, but please check my change in case I'm wrong here.

sycl/test-e2e/README.md

aelovikov-intel · 2023-04-11T17:29:18Z

CI Failures:

L0 GEN9 Linux - not started, infrastructure problem?
OCL GEN9/CPU:
SYCL :: Regression/device_num.cpp

/__w/llvm/llvm/llvm/sycl/test-e2e/Regression/device_num.cpp:29:15: error: no member named 'ext_oneapi_unified_runtime' in 'sycl::backend'
    {backend::ext_oneapi_unified_runtime, "ext_oneapi_unified_runtime"},
     ~~~~~~~~~^

could it be caused by #9013? @smaslov-intel @jandres742
EDIT: I believe the issue is caused by https://github.com/intel/llvm/pull/8980/files#r1160229091 @bader @uditagarwal97 , JFYI.

ESIMD: known issue, investigation is being tracked in an internal bug report
Windows: Likely known issue with a mis-configured runner.

@intel/llvm-gatekeepers , failures seem unrelated, can we merge this PR?

jandres742 · 2023-04-11T17:37:31Z

@aelovikov-intel : right, please rebase so you incorporate those changes and that error should go away.

This provides the added benefit that it will automatically map the config paths from the build directories to the corresponding source ones in `llvm-lit`. With this, one should be able to run lit tests directly from source paths. In standalone builds, we don't have access to LLVM's wider configuration, so the old configuration must remain. Note also that this changes the configured cfg file's name from `lit.site.cfg` to `lit.site.cfg.py`, but this aligns us with the predominant style across the llvm project.

frasercrmck · 2023-04-12T10:21:39Z

Looks like most of the failures have indeed gone away, except for Windows L0 GEN12

steffenlarsen · 2023-04-12T10:45:06Z

Unexpectedly Passed Tests (1):
SYCL :: ESIMD/thread_id_test.cpp - Seen on other PRs. According to the comments in the test, this may be expected. @fineg74 for awareness.

aelovikov-intel · 2023-04-13T18:48:10Z

sycl/test-e2e/README.md

+# Implicitly uses cmake parameters SYCL_BE and SYCL_TARGET_DEVICES, detailed
+# below


It seems it requires specifying them manually, in addition to SYCL_TEST_E2E_TARGETS above. Maybe worth adding a note somewhere here?

After intel#8854 it's possible to run llvm-lit from anywhere and not be restricted to the end-to-end tests' build directory. However, lit.cfg.py performs auto-detection of some features by creating and compiling small test files. Make sure that those are kept in the build directory no matter where we run llvm-lit from.

…9078) After #8854 it's possible to run llvm-lit from anywhere and not be restricted to the end-to-end tests' build directory. However, lit.cfg.py performs auto-detection of some features by creating and compiling small test files. Make sure that those are kept in the build directory no matter where we run llvm-lit from.

Since intel#9078 (itself a fix for intel#8854), the SYCL E2E lit config would change the working directory and never set it back. This impeded LIT's ability to discover tests and test suites, when tasked with sourcing multiple. For example: llvm-lit sycl/test/A sycl/test/B LIT would discover test 'A' by appending its relative path to the CWD (e.g., root/sycl/test/A) and load up the SYCL E2E lit.cfg. This would change directory into the SYCL build directory. When discovering test 'B' it would append its relative path to the SYCL binary dir and fail to find it (e.g., root/build/tools/sycl/test-e2e/sycl/test/B). This would also have the effect of load the SYCL E2E LIT config a second time, even though it was already loaded. With this change, we maintain the CWD but open the temp files using absolute paths. This allows us to run multiple tests from multiple different paths at the same time, even ones from different suites: llvm-lit sycl/test/A clang/test/B llvm/test/C Running multiple individual SYCL tests on the same command line will also only load the SYCL config the once.

Since #9078 (itself a fix for #8854), the SYCL E2E lit config would change the working directory and never set it back. This impeded LIT's ability to discover tests and test suites, when tasked with sourcing multiple. For example: llvm-lit sycl/test/A sycl/test/B LIT would discover test 'A' by appending its relative path to the CWD (e.g., `root/sycl/test/A`) and load up the SYCL E2E lit.cfg. This would change directory into the SYCL build directory. When discovering test 'B' it would append its relative path to the SYCL binary dir and fail to find it (e.g., `root/build/tools/sycl/test-e2e/sycl/test/B`). This would also have the effect of loading the SYCL E2E LIT config a second time, even though it was already loaded. With this change, we maintain the CWD but open the temp files using absolute paths. This allows us to run multiple tests from multiple different paths at the same time, even ones from different suites: llvm-lit sycl/test/A clang/test/B llvm/test/C Running multiple individual SYCL tests on the same command line will also only load the SYCL config the once.

frasercrmck requested a review from a team as a code owner March 29, 2023 09:38

frasercrmck requested a review from cperkinsintel March 29, 2023 09:38

frasercrmck temporarily deployed to aws March 29, 2023 10:09 — with GitHub Actions Inactive

frasercrmck temporarily deployed to aws March 29, 2023 11:37 — with GitHub Actions Inactive

frasercrmck changed the title ~~[SYCL] Use configure_lit_site_cfg in test-e2e~~ [SYCL][E2E] Use configure_lit_site_cfg Apr 10, 2023

frasercrmck force-pushed the configure-lit-site-cfg branch from 03b7b93 to 576994b Compare April 10, 2023 09:46

frasercrmck temporarily deployed to aws April 10, 2023 10:11 — with GitHub Actions Inactive

frasercrmck temporarily deployed to aws April 10, 2023 10:13 — with GitHub Actions Inactive

frasercrmck force-pushed the configure-lit-site-cfg branch from 576994b to cad7849 Compare April 11, 2023 09:29

frasercrmck force-pushed the configure-lit-site-cfg branch from cad7849 to 66f5dce Compare April 11, 2023 09:41

frasercrmck changed the title ~~[SYCL][E2E] Use configure_lit_site_cfg~~ [SYCL][E2E] Use configure_lit_site_cfg in in-tree builds Apr 11, 2023

frasercrmck temporarily deployed to aws April 11, 2023 10:34 — with GitHub Actions Inactive

aelovikov-intel approved these changes Apr 11, 2023

View reviewed changes

sycl/test-e2e/README.md Show resolved Hide resolved

sycl/test-e2e/README.md Outdated Show resolved Hide resolved

frasercrmck temporarily deployed to aws April 11, 2023 16:41 — with GitHub Actions Inactive

frasercrmck force-pushed the configure-lit-site-cfg branch from 66f5dce to 721a2ad Compare April 12, 2023 09:01

frasercrmck temporarily deployed to aws April 12, 2023 09:32 — with GitHub Actions Inactive

frasercrmck temporarily deployed to aws April 12, 2023 10:17 — with GitHub Actions Inactive

steffenlarsen merged commit 008b0d1 into intel:sycl Apr 12, 2023

aelovikov-intel reviewed Apr 13, 2023

View reviewed changes

aelovikov-intel mentioned this pull request Apr 14, 2023

[SYCL][E2E LIT] Don't spoil current directory with temporary files #9078

Merged

frasercrmck deleted the configure-lit-site-cfg branch June 7, 2023 14:34

frasercrmck mentioned this pull request Jul 11, 2024

[SYCL][E2E] Fix multiple test suite discovery #14534

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SYCL][E2E] Use configure_lit_site_cfg in in-tree builds #8854

[SYCL][E2E] Use configure_lit_site_cfg in in-tree builds #8854

Uh oh!

frasercrmck commented Mar 29, 2023

Uh oh!

frasercrmck commented Apr 10, 2023

Uh oh!

aelovikov-intel commented Apr 10, 2023

Uh oh!

aelovikov-intel commented Apr 10, 2023

Uh oh!

frasercrmck commented Apr 11, 2023

Uh oh!

Uh oh!

Uh oh!

aelovikov-intel commented Apr 11, 2023 •

edited

Loading

Uh oh!

jandres742 commented Apr 11, 2023

Uh oh!

frasercrmck commented Apr 12, 2023

Uh oh!

steffenlarsen commented Apr 12, 2023

Uh oh!

aelovikov-intel Apr 13, 2023

Uh oh!

Uh oh!

		# Implicitly uses cmake parameters SYCL_BE and SYCL_TARGET_DEVICES, detailed
		# below

[SYCL][E2E] Use configure_lit_site_cfg in in-tree builds #8854

[SYCL][E2E] Use configure_lit_site_cfg in in-tree builds #8854

Uh oh!

Conversation

frasercrmck commented Mar 29, 2023

Uh oh!

frasercrmck commented Apr 10, 2023

Uh oh!

aelovikov-intel commented Apr 10, 2023

Uh oh!

aelovikov-intel commented Apr 10, 2023

Uh oh!

frasercrmck commented Apr 11, 2023

Uh oh!

Uh oh!

Uh oh!

aelovikov-intel commented Apr 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jandres742 commented Apr 11, 2023

Uh oh!

frasercrmck commented Apr 12, 2023

Uh oh!

steffenlarsen commented Apr 12, 2023

Uh oh!

aelovikov-intel Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aelovikov-intel commented Apr 11, 2023 •

edited

Loading