Skip to content

Conversation

@aelovikov-intel
Copy link
Contributor

...and use it to pass amd_arch options to compilation only when
compiling for AMD triples instead of unconditional usage in %clangxx
substitution.

That would avoid passing the option unnecessary when running llvm-lit
with multiple target devices and a given test isn't supported for HIP.
We might not have such configuration presently, but the same logic will
be applied for "split" build/run of E2E tests and we'd definitely want
to build tests for all the possible targets at once.

Another use case for it is the "spirv backend" testing where we'd want
to pass extra option for any compilations targeting spirv triple.

...and use it to pass `amd_arch` options to compilation only when
compiling for AMD triples instead of unconditional usage in `%clangxx`
substitution.

That would avoid passing the option unnecessary when running `llvm-lit`
with multiple target devices and a given test isn't supported for HIP.
We might not have such configuration presently, but the same logic will
be applied for "split" build/run of E2E tests and we'd definitely want
to build tests for all the possible targets at once.

Another use case for it is the "spirv backend" testing where we'd want
to pass extra option for any compilations targeting spirv triple.
Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

nice!

Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

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

LGTM.

@aelovikov-intel
Copy link
Contributor Author

@intel/syclcompat-lib-reviewers , ping.

@aelovikov-intel aelovikov-intel merged commit df91a6b into intel:sycl Jan 15, 2025
17 checks passed
@aelovikov-intel aelovikov-intel deleted the e2e_sycl_target_opts branch January 15, 2025 17:07
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