Skip to content

[SYCL][Driver] Enable SPV_INTEL_memory_access_aliasing extension #14992

New issue

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

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

Already on GitHub? # to your account

Merged
merged 3 commits into from
Aug 20, 2024

Conversation

maarquitos14
Copy link
Contributor

No description provided.

Signed-off-by: Marcos Maronas <marcos.maronas@intel.com>
Signed-off-by: Marcos Maronas <marcos.maronas@intel.com>
@maarquitos14 maarquitos14 requested a review from a team as a code owner August 7, 2024 16:05
@mdtoguchi
Copy link
Contributor

@maarquitos14, will there be additional changes done to the clang-linker-wrapper to add this extension for the new offloading model?

@maarquitos14
Copy link
Contributor Author

@maarquitos14, will there be additional changes done to the clang-linker-wrapper to add this extension for the new offloading model?

Tbh, I just adapted #4025 to the current codebase. What additional changes do we need to add this extension for the new offloading model?

@mdtoguchi
Copy link
Contributor

@maarquitos14, will there be additional changes done to the clang-linker-wrapper to add this extension for the new offloading model?

Tbh, I just adapted #4025 to the current codebase. What additional changes do we need to add this extension for the new offloading model?

Perform a similar addition to

getTripleBasedSPIRVTransOpts(const ArgList &Args,
should take care of it

Signed-off-by: Marcos Maronas <marcos.maronas@intel.com>
@maarquitos14 maarquitos14 requested a review from a team as a code owner August 7, 2024 18:06
@maarquitos14
Copy link
Contributor Author

@maarquitos14, will there be additional changes done to the clang-linker-wrapper to add this extension for the new offloading model?

Tbh, I just adapted #4025 to the current codebase. What additional changes do we need to add this extension for the new offloading model?

Perform a similar addition to

getTripleBasedSPIRVTransOpts(const ArgList &Args,

should take care of it

Done :)

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.

Thanks - OK for driver. Is there some test for the extensions for clang-linker-wrapper?

@maarquitos14
Copy link
Contributor Author

Thanks - OK for driver. Is there some test for the extensions for clang-linker-wrapper?

I checked, but didn't find any. Glad to update it if I missed it.

@AlexeySachkov
Copy link
Contributor

Thanks - OK for driver. Is there some test for the extensions for clang-linker-wrapper?

I checked, but didn't find any. Glad to update it if I missed it.

@maksimsab, @asudarsa, do you know?

@maksimsab
Copy link
Contributor

Thanks - OK for driver. Is there some test for the extensions for clang-linker-wrapper?

It looks like we are missing such a test.

@maarquitos14
Copy link
Contributor Author

Thanks - OK for driver. Is there some test for the extensions for clang-linker-wrapper?

It looks like we are missing such a test.

@mdtoguchi @AlexeySachkov should I add it here or should that be addressed in a different PR?

@maarquitos14
Copy link
Contributor Author

@mdtoguchi @AlexeySachkov ping

@AlexeySachkov
Copy link
Contributor

AlexeySachkov commented Aug 14, 2024

Thanks - OK for driver. Is there some test for the extensions for clang-linker-wrapper?

It looks like we are missing such a test.

@mdtoguchi @AlexeySachkov should I add it here or should that be addressed in a different PR?

Fine by me to have it in a separate PR. Especially considering that longer term clang-linker-wrapper should not use the translator as a binary, but instead should use it through a library call

@maarquitos14
Copy link
Contributor Author

@intel/llvm-gatekeepers I think this is ready to merge :)

@sommerlukas sommerlukas merged commit 0a9db37 into intel:sycl Aug 20, 2024
13 checks passed
jsji added a commit that referenced this pull request Aug 21, 2024
@sarnex
Copy link
Contributor

sarnex commented Aug 26, 2024

@maarquitos14 I'm pretty sure you're already working on this but this broke all invokesimd tests (~55 tests) on dev igc arc, so I'm going to revert this, discussed with Jinsong.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants