Skip to content

Conversation

aelovikov-intel
Copy link
Contributor

@aelovikov-intel aelovikov-intel commented Jul 31, 2025

Development happens mostly upstream, so there is no need to have that in pre-commit, post-commit should be enough.

Before this PR, when running E2E tests in "in-tree" mode (as opposite to a standalone e2e tests cmake invocation), some utilities (e.g. llvm-ar) were only searched for in PATH that didn't contain freshly built toolchain. Our pre-commit used a container image with last successful "Nightly" toolchain installed, so those were picked up from there. Post-commit uses GNU Compiler instead of Clang, so these utilities aren't available in PATH.

This PR modifieslit.cfg.py such that we search for those utilities in the newly built toolchain first (still not modifying PATH which seems to be the approach taken in LIT). That also ensures that opencl-aot feature is available when running E2E tests in build-only mode in Post-commit's build job.

@aelovikov-intel aelovikov-intel force-pushed the move-spirv-backend-e2e branch from 490abfa to 20ace65 Compare July 31, 2025 18:55
@aelovikov-intel aelovikov-intel force-pushed the move-spirv-backend-e2e branch from 20ace65 to 0ee77dc Compare July 31, 2025 20:57
@aelovikov-intel aelovikov-intel force-pushed the move-spirv-backend-e2e branch from 0ee77dc to fac3809 Compare July 31, 2025 22:01
@aelovikov-intel aelovikov-intel force-pushed the move-spirv-backend-e2e branch from fac3809 to 0bc6a7b Compare July 31, 2025 22:23
@sarnex sarnex requested a review from a team August 1, 2025 18:45
@aelovikov-intel
Copy link
Contributor Author

Ping @againull , @reble , @intel/sycl-graphs-reviewers .

Copy link
Contributor

@againull againull left a comment

Choose a reason for hiding this comment

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

Initially I assumed this was already approved on behalf of llvm-reviewers-runtime

@sarnex
Copy link
Contributor

sarnex commented Aug 4, 2025

Yeah sorry I'm not really a runtime team member so I usually re-add the team to reviews whenever my review is used for runtime, I'm in the group for CI/test reasons

@againull
Copy link
Contributor

againull commented Aug 4, 2025

Yeah sorry I'm not really a runtime team member so I usually re-add the team to reviews whenever my review is used for runtime, I'm in the group for CI/test reasons

No worries, that's my bad

@aelovikov-intel
Copy link
Contributor Author

@reble , @intel/sycl-graphs-reviewers , ping. The change in the graphs tests is minimal and shouldn't take more than a few minutes to review.

Copy link
Member

@adamfidel adamfidel left a comment

Choose a reason for hiding this comment

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

LGTM. Apologies for the slow review.

@aelovikov-intel
Copy link
Contributor Author

Windows failure is #19660, unrelated.

@aelovikov-intel aelovikov-intel merged commit 35f944f into intel:sycl Aug 5, 2025
34 of 35 checks passed
@aelovikov-intel aelovikov-intel deleted the move-spirv-backend-e2e branch August 5, 2025 14:47
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.

4 participants