Skip to content

[SYCL] Address multi_ptr intel/llvm vs spec mismatches. #10297

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.

Sign up for GitHub

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

Jump to bottom
Merged
steffenlarsen merged 13 commits into from
Jul 17, 2023
Merged

[SYCL] Address multi_ptr intel/llvm vs spec mismatches. #10297

steffenlarsen merged 13 commits into from
Jul 17, 2023

Conversation

@mmoadeli
Copy link
Contributor

@mmoadeli mmoadeli commented Jul 11, 2023 •
edited
Loading

  • Deprecate the multi_ptr constructor from accessor(Reg, Void) with target::local.
  • Deprecate the multi_ptr constructor from accessor(Void) and local_accessor with generic_space.
  • Deprecate the multi_ptr constructor from Legacy using local_accessor.
  • Add const qualification to the explicit conversion of multi_ptr<generic_space> to multi_ptr of any other address space.
  • Adjust the order of template parameter deduction guides in sync with the specification.
  • Implement support for the constant_space specialization of non-legacy multi_ptr.

@mmoadeli
Address multi_ptr impl / spec mismatches.
23ac71e 
- Deprecation of multi_ptr ctor from accessor(Reg, Void) with target::local
- ctor from accessor(Void) and local_accessor with generic_space
- Deprecation of multi_ptr<legacy>
- ctor(Legacy) from local_accessor
- Add const qualified to explicit conversion of multi_ptr<generic_space> to multi_ptr of any other address space.
@mmoadeli mmoadeli requested a review from a team as a code owner July 11, 2023 11:51
@mmoadeli mmoadeli requested a review from againull July 11, 2023 11:51
@mmoadeli mmoadeli changed the title Address multi_ptr impl / spec mismatches. Address multi_ptr intel/llvm vs spec mismatches. Jul 11, 2023
@mmoadeli mmoadeli temporarily deployed to aws July 11, 2023 12:01 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 11, 2023 12:42 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 11, 2023 13:11 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 11, 2023 14:13 — with GitHub Actions Inactive
@mmoadeli mmoadeli changed the title Address multi_ptr intel/llvm vs spec mismatches. [SYCL] Address multi_ptr intel/llvm vs spec mismatches. Jul 12, 2023
@mmoadeli mmoadeli changed the title [SYCL] Address multi_ptr intel/llvm vs spec mismatches. [SYCL][ABI-Break] Address multi_ptr intel/llvm vs spec mismatches. Jul 12, 2023
@mmoadeli
Replace access::decorated::legacy to access::decorated::no in deducti…
d3f31a2 
…on guide from constant_buffer specialised accessor to conform with the spec.
@mmoadeli mmoadeli temporarily deployed to aws July 13, 2023 22:05 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 13, 2023 23:20 — with GitHub Actions Inactive
@mmoadeli mmoadeli requested a review from steffenlarsen July 14, 2023 10:02
@againull
Copy link
Contributor

againull commented Jul 14, 2023 •
edited
Loading

@mmoadeli Could you please fix LIT failures.
At least this one in WIndows testing looks relevant:
Failed Tests (1):
SYCL :: multi_ptr/ctad.cpp

againull
againull reviewed Jul 14, 2023
View reviewed changes
@againull
Copy link
Contributor

againull commented Jul 14, 2023

Could you please merge origin/sycl branch because some of the tests failed because of misalignment between test-e2e and compiler (see #10334 for details)

@mmoadeli mmoadeli changed the title [SYCL][ABI-Break] Address multi_ptr intel/llvm vs spec mismatches. [SYCL] Address multi_ptr intel/llvm vs spec mismatches. Jul 14, 2023
@mmoadeli mmoadeli temporarily deployed to aws July 14, 2023 13:56 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 14, 2023 15:00 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 14, 2023 16:34 — with GitHub Actions Inactive
@againull
Copy link
Contributor

againull commented Jul 14, 2023 •
edited
Loading

@mmoadeli Could you please fix the following failure:
Failed Tests (1):
SYCL :: multi_ptr/ctad.cpp

@againull
Copy link
Contributor

againull commented Jul 14, 2023 •
edited
Loading

@mmoadeli Could you please fix the following failure: Failed Tests (1): SYCL :: multi_ptr/ctad.cpp

This failure is caused by this commit: d3f31a2

Could you please point me to the specification where I can see this deduction rule?

@againull
Copy link
Contributor

againull commented Jul 14, 2023 •
edited
Loading

@mmoadeli Could you please fix the following failure: Failed Tests (1): SYCL :: multi_ptr/ctad.cpp

This failure is caused by this commit: d3f31a2

Could you please point me to the specification where I can see this deduction rule?
It looks before this change legacy specialization of multi_ptr was picked. But after this change (decoration changed from legacy to "no") SYCL 2020 multi_ptr is picked and assert is thrown because SYCL 2020 multi_ptr doesn't allow constant_buffer.

@mmoadeli
Copy link
Contributor Author

mmoadeli commented Jul 14, 2023

@mmoadeli mmoadeli reopened this Jul 14, 2023
@mmoadeli
Copy link
Contributor Author

mmoadeli commented Jul 14, 2023 •
edited
Loading

@mmoadeli Could you please fix the following failure: Failed Tests (1): SYCL :: multi_ptr/ctad.cpp

This failure is caused by this commit: d3f31a2

Could you please point me to the specification where I can see this deduction rule? It looks before this change legacy specialization of multi_ptr was picked. But after this change (decoration changed from legacy to "no") SYCL 2020 multi_ptr is picked and assert is thrown because SYCL 2020 multi_ptr doesn't allow constant_buffer.

@againull This change is not still part of spec. There is a pending PR for it

@mmoadeli mmoadeli temporarily deployed to aws July 14, 2023 23:00 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 14, 2023 23:13 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 15, 2023 00:12 — with GitHub Actions Inactive
@againull againull self-requested a review July 15, 2023 00:45
@mmoadeli mmoadeli temporarily deployed to aws July 15, 2023 02:05 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 15, 2023 02:09 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 15, 2023 02:44 — with GitHub Actions Inactive
@mmoadeli
- Allow constant_space specialisation of non-legacy multi_ptr.
8a1f1a6 
- Deduction guide construct decorated::no specialised multi_ptr for constant_buffer specialized accessors.
@mmoadeli mmoadeli temporarily deployed to aws July 15, 2023 18:20 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 15, 2023 19:00 — with GitHub Actions Inactive
@mmoadeli
- Declare non-legacy constant_space specialised multi_ptr as deprecated.
1fa551a 
- Replace deprecated access::target with target.
@mmoadeli mmoadeli temporarily deployed to aws July 16, 2023 19:29 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 16, 2023 20:07 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 16, 2023 21:17 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 16, 2023 21:18 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 16, 2023 21:57 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 16, 2023 21:57 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 17, 2023 09:05 — with GitHub Actions Inactive
@mmoadeli mmoadeli temporarily deployed to aws July 17, 2023 09:13 — with GitHub Actions Inactive
@steffenlarsen steffenlarsen merged commit ed5941f into intel:sycl Jul 17, 2023
@mmoadeli mmoadeli deleted the spec-impl-mismatch branch July 17, 2023 21:18
YuriPlyakhin added a commit to YuriPlyakhin/llvm that referenced this pull request Jul 26, 2023
@YuriPlyakhin
[SYCL][Joint Matrix] Resolve non-standard usage of multi_ptr
45c8325 
Fix for use of non-standard multi_ptr constructors
in several Joint Matrix tests exposed by intel#10297
@YuriPlyakhin YuriPlyakhin mentioned this pull request Jul 26, 2023
Merged
aelovikov-intel pushed a commit that referenced this pull request Aug 1, 2023
@YuriPlyakhin
[SYCL][Joint Matrix] Resolve non-standard usage of multi_ptr (#10583)
f6eeb67 
Fix for use of non-standard multi_ptr constructors in several Joint
Matrix tests exposed by #10297
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
@YuriPlyakhin @mdtoguchi
[SYCL][Joint Matrix] Resolve non-standard usage of multi_ptr (intel#1…
eee1c8e 
…0583)

Fix for use of non-standard multi_ptr constructors in several Joint
Matrix tests exposed by intel#10297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@againull againull againull approved these changes

@steffenlarsen steffenlarsen Awaiting requested review from steffenlarsen

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mmoadeli @againull @steffenlarsen