Skip to content

[SYCL] Fix triple in offload mismatch warning #10385

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 2 commits into from
Jul 28, 2023

Conversation

npmiller
Copy link
Contributor

This patch fixes small issues after #9631

When standardizing the triples the format should be:

<arch><sub>-<vendor>-<sys>-<env>

And then we add our offload arch afterwards, so the final triples should look something like: nvptx64-nvidia-cuda--sm_50, instead of nvptx64-nvidia-cuda-sm_50-.

In addition never skip standardization so that triples like nvptx64-nvidia-cuda-, are also properly standardized to nvptx64-nvidia-cuda--.

When standardizing the triples the format should be:

```
<arch><sub>-<vendor>-<sys>-<env>
```

* https://clang.llvm.org/docs/CrossCompilation.html#target-triple

And then we add our offload arch afterwards, so the final triples should
look something like: `nvptx64-nvidia-cuda--sm_50`, instead of
`nvptx64-nvidia-cuda-sm_50-`.

In addition never skip standardization so that triples like
`nvptx64-nvidia-cuda-`, are also properly standardized to
`nvptx64-nvidia-cuda--`.
@npmiller npmiller requested a review from a team as a code owner July 14, 2023 14:21
@npmiller npmiller temporarily deployed to aws July 14, 2023 14:49 — with GitHub Actions Inactive
@mdtoguchi mdtoguchi requested a review from asudarsa July 14, 2023 15:25
@npmiller npmiller temporarily deployed to aws July 14, 2023 16:13 — with GitHub Actions Inactive
Copy link
Contributor

@mdtoguchi mdtoguchi left a comment

Choose a reason for hiding this comment

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

LGTM, would like a look from @asudarsa as he made the original changes

Copy link
Contributor

@asudarsa asudarsa left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix.

@asudarsa
Copy link
Contributor

Hi @npmiller

Thanks for the fix again. can you please sync your PR to get latest changes? The fix to the build issue was checked in yesterday and so the testing after the sync should be useful.

@npmiller npmiller temporarily deployed to aws July 21, 2023 08:22 — with GitHub Actions Inactive
@npmiller npmiller temporarily deployed to aws July 21, 2023 09:00 — with GitHub Actions Inactive
@asudarsa
Copy link
Contributor

@AlexeySachkov

Can you please take a look and merge if it looks ok to you?

Thanks
Sincerely

@AlexeySachkov AlexeySachkov merged commit 0fd9a4e into intel:sycl Jul 28, 2023
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
This patch fixes small issues after
intel#9631

When standardizing the triples the format should be:

```
<arch><sub>-<vendor>-<sys>-<env>
```

* https://clang.llvm.org/docs/CrossCompilation.html#target-triple

And then we add our offload arch afterwards, so the final triples should
look something like: `nvptx64-nvidia-cuda--sm_50`, instead of
`nvptx64-nvidia-cuda-sm_50-`.

In addition never skip standardization so that triples like
`nvptx64-nvidia-cuda-`, are also properly standardized to
`nvptx64-nvidia-cuda--`.
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