Skip to content

[Driver][SYCL] Do not emit mismatch warning with -fsycl-force-target #17013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 17, 2025

Conversation

mdtoguchi
Copy link
Contributor

@mdtoguchi mdtoguchi commented Feb 14, 2025

The use of -fsycl-force-target=arg allows for a user to override the default target triple that is used to unbundle device objects from the fat objects. A diagnostic warning is emitted to inform the user that the expected target values within the given objects is not found. This diagnostic is not valid when -fsycl-force-target is used with the matching target value.

Avoid emitting this warning when the -fsycl-force-target=arg matches what is found in the incoming objects.

The use of -fsycl-force-target=arg allows for a user to override the
default target triple that is used to unbundle device objects from the
fat objects.  The diagnostic warning a user that the expected target
values within the given objects is not found is not valid when
-fsycl-force-target is used with the matching target value.

Avoid emitting this warning when the -fsycl-force-target=arg matches
what is found in the incoming objects.
@mdtoguchi mdtoguchi requested a review from a team as a code owner February 14, 2025 00:21
@srividya-sundaram
Copy link
Contributor

The diagnostic warning a user that the expected target values within the given objects is not found is not valid when -fsycl-force-target is used with the matching target value.

This sentence is hard to understand with the current wording.
Could you rephrase it to make it clearer?

@mdtoguchi
Copy link
Contributor Author

The diagnostic warning a user that the expected target values within the given objects is not found is not valid when -fsycl-force-target is used with the matching target value.

This sentence is hard to understand with the current wording. Could you rephrase it to make it clearer?

Wow - the original description was quite the word jumble. I have updated for clarity.

@@ -22,4 +22,7 @@
// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen %S/Inputs/SYCL/liblin64.a \
// RUN: -Wno-sycl-target -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=SPIR64_DIAG
// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen %S/Inputs/SYCL/liblin64.a \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think passing %S/Inputs/SYCL/liblin64.a and ### is not necessary here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The passing of liblin64.a is necessary, it represents the binary that contains the spir64-unknown-unknown bundled fat object that should trigger the diagnostic due to -fsycl-targets=spir64_gen.

Copy link
Contributor

Choose a reason for hiding this comment

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

From the documentation , my understanding is, when we invoke,

icx -fsycl -fsycl-targets=spir64_gen -fsycl-force-target=spir64

spir64 objects/archives will be extracted but spir64_gen targets will still compile.

I dont see the spir64 archive explicitly passed to the compiler.

Also, since we are checking if a diagnostic is being thrown or not, do we need to pass -### ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The documentation doesn't show a full command line, it just demonstrates how the options work together - it probably should be updated to be a little clearer.

The use of -### ensures that only the driver is invoked here. Without it, we would run additional tools that aren't necessary since we are only checking a diagnostic from the driver.

@mdtoguchi
Copy link
Contributor Author

@intel/llvm-gatekeepers, this is ready for merge, thanks!

@martygrant martygrant merged commit 0dfb947 into intel:sycl Feb 17, 2025
30 checks passed
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