Skip to content

[SYCL][NativeCPU] Copy over more host/aux target data. #17999

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 6 commits into from
Apr 22, 2025

Conversation

hvdijk
Copy link
Contributor

@hvdijk hvdijk commented Apr 14, 2025

  • Use copyAuxTarget instead of copying over each field.
  • Copy over TargetOpts as well.
  • Prevent recording the target flags for builtins.

* Use copyAuxTarget instead of copying over each field.
* Copy over TargetOpts as well.
* Prevent recording the target flags for builtins.
@hvdijk hvdijk requested review from a team as code owners April 14, 2025 12:27
@hvdijk hvdijk requested a review from npmiller April 14, 2025 12:27
@hvdijk hvdijk temporarily deployed to WindowsCILock April 14, 2025 12:27 — with GitHub Actions Inactive
@hvdijk
Copy link
Contributor Author

hvdijk commented Apr 14, 2025

Note since it may not be obvious why it's part of this PR: The "Prevent recording the target flags for builtins" needs to be part of this because the additional data that gets recorded would otherwise start including target-specific flags in the builtins, preventing them from being used for cross-compilation.

Copy link
Contributor

@Fznamznon Fznamznon left a comment

Choose a reason for hiding this comment

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

Does it fix any bugs? If so, it needs a test. If not, it needs a NFC tag.

@hvdijk hvdijk temporarily deployed to WindowsCILock April 18, 2025 12:47 — with GitHub Actions Inactive
@hvdijk
Copy link
Contributor Author

hvdijk commented Apr 18, 2025

Does it fix any bugs? If so, it needs a test. If not, it needs a NFC tag.

The main goal is to ensure target flags are recorded in the device code in the same way that they are recorded for host code, I've added a test for that, thanks for the suggestion.

@hvdijk
Copy link
Contributor Author

hvdijk commented Apr 22, 2025

@intel/llvm-gatekeepers This can be merged, thanks.

@ldrumm ldrumm merged commit d653b20 into intel:sycl Apr 22, 2025
28 of 30 checks passed
aelovikov-intel pushed a commit that referenced this pull request May 15, 2025
In #17999 I changed prepare-builtins to clear target-cpu and
target-features for NativeCPU, and attempted to preserve existing
behavior for other targets. For NativeCPU, two errors cancelled each
other out so we do the right thing there, but for other targets, that is
not the case and target-cpu and target-features were also being cleared
unintentionally.
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.

6 participants