Skip to content

Conversation

frasercrmck
Copy link
Contributor

@frasercrmck frasercrmck commented Apr 12, 2023

This change makes LIT explicitly substitute uses of tool binaries in
tests with their full paths. This makes test RUN lines easier to
reproduce, as it negates the effect of lit's internal environment (e.g.,
PATH) on which tools are run.

The list of tools affected are: sycl-ls llvm-spirv, llvm-link, FileCheck
and not (when used after a pipe).

This also changes the behaviour of how LIT finds sycl-ls, llvm-spirv,
and llvm-link. Whereas before these tools were found only on the PATH,
now they will be preferably sourced first from the in-tree tools
directory.

@frasercrmck frasercrmck requested a review from a team as a code owner April 12, 2023 10:41
@frasercrmck
Copy link
Contributor Author

@aelovikov-intel might be of interest to you again.

@frasercrmck frasercrmck temporarily deployed to aws April 12, 2023 11:39 — with GitHub Actions Inactive
@frasercrmck
Copy link
Contributor Author

frasercrmck commented Apr 12, 2023

Hmm, I now see that pre-built tools should already be picked up in tests by this line:

llvm_config.with_environment('PATH', config.sycl_tools_dir, append_path=True)

but later on in the cfg we still have manual checks and warnings for llvm-spirv and llvm-link which don't take this into account, so we're unnecessarily disabling tests if these tools are present in the tools directory but aren't on the PATH.

@frasercrmck frasercrmck temporarily deployed to aws April 12, 2023 12:26 — with GitHub Actions Inactive
@frasercrmck frasercrmck changed the title [SYCL][E2E] Attempt to find lit tool binaries in-tree [SYCL][E2E] Substitute paths to lit tool binaries Apr 13, 2023
@frasercrmck frasercrmck temporarily deployed to aws April 13, 2023 11:08 — with GitHub Actions Inactive
@frasercrmck frasercrmck temporarily deployed to aws April 13, 2023 11:48 — with GitHub Actions Inactive
@frasercrmck frasercrmck temporarily deployed to aws April 13, 2023 17:24 — with GitHub Actions Inactive
@frasercrmck frasercrmck temporarily deployed to aws April 13, 2023 19:02 — with GitHub Actions Inactive
This change makes LIT explicitly substitute uses of tool binaries in
tests with their full paths. This makes test RUN lines easier to
reproduce, as it negates the effect of lit's internal environment (e.g.,
PATH) on which tools are run.

The list of tools affected are: sycl-ls llvm-spirv, llvm-link, FileCheck
and not (when used after a pipe).

This also changes the behaviour of how LIT finds sycl-ls, llvm-spirv,
and llvm-link. Whereas before these tools were found only on the PATH,
now they will be preferably sourced first from the in-tree tools
directory.
@frasercrmck frasercrmck temporarily deployed to aws April 17, 2023 10:27 — with GitHub Actions Inactive
@frasercrmck
Copy link
Contributor Author

I'm not sure if it's normal to wait over 6 hours for an AMGPU runner but rebasing this PR looks to have squashed all of the failures we were previously seeing.

@frasercrmck frasercrmck temporarily deployed to aws April 17, 2023 18:22 — with GitHub Actions Inactive
@aelovikov-intel
Copy link
Contributor

@intel/llvm-gatekeepers , this PR is ready.

@steffenlarsen steffenlarsen merged commit 3467a00 into intel:sycl Apr 18, 2023
@frasercrmck frasercrmck deleted the lit-e2e-tool-paths branch June 7, 2023 14:35
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