Skip to content

[SYCL-MLIR] Opaque pointer handling #8616

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
Closed
whitneywhtsang opened this issue Mar 10, 2023 · 7 comments
Closed

[SYCL-MLIR] Opaque pointer handling #8616

whitneywhtsang opened this issue Mar 10, 2023 · 7 comments
Assignees
@sommerlukas @victor-eds @whitneywhtsang @etiotto
Labels
enhancement New feature or request sycl-mlir Pull requests or issues for sycl-mlir branch

Comments

@whitneywhtsang
Copy link
Contributor

whitneywhtsang commented Mar 10, 2023 •
edited
Loading

  1. Add back 02f9cf9, which was reverted in [SYCL-MLIR] Merge from intel/llvm sycl branch #8600.
  2. Add back 130466e, which was reverted in [SYCL-MLIR] Merge from intel/llvm sycl branch #8774.
  3. Add back 946f803, which was reverted in [SYCL-MLIR] Merge from intel/llvm sycl branch #8849.
  4. Revert 479c2cc, added in [SYCL-MLIR] Merge from intel/llvm sycl branch #10724.
  5. Modify test cases to use opaque pointers.
  6. Performance testing with using opaque pointers.
  7. Implementation may need to changed, e.g., TBAA for SYCL operations.
@whitneywhtsang whitneywhtsang added enhancement New feature or request sycl-mlir Pull requests or issues for sycl-mlir branch labels Mar 10, 2023
@whitneywhtsang whitneywhtsang mentioned this issue Mar 10, 2023
Merged
@sommerlukas sommerlukas mentioned this issue Mar 24, 2023
Merged
@whitneywhtsang whitneywhtsang mentioned this issue Mar 24, 2023
Closed
sommerlukas added a commit that referenced this issue Mar 28, 2023
@sommerlukas
[SYCL-MLIR] Opaque pointer support Polygeist-to-LLVM (#8767)
b1e03ed 
First step to add opaque pointer support for the SYCL-MLIR project: Make
the lowering patterns in the Polygeist-to-LLVM lowering compatible with
opaque pointers.

Partly resolves #8616.

---------

Signed-off-by: Lukas Sommer <[email protected]>
@whitneywhtsang whitneywhtsang mentioned this issue Mar 29, 2023
Merged
@sommerlukas sommerlukas mentioned this issue Apr 4, 2023
Merged
sommerlukas added a commit that referenced this issue Apr 5, 2023
@sommerlukas
[SYCL-MLIR] Opaque pointer support in SYCL-to-LLVM conversion (#8944)
001a03c 
Second step to add opaque pointer support for the SYCL-MLIR project: Make
the lowering patterns in the SYCL-to-LLVM lowering compatible with
opaque pointers.

Emitting typed or opaque pointers is controlled by the
`use-opaque-pointers` pass option, that was added.

Partly resolves #8616.

---------

Signed-off-by: Lukas Sommer <[email protected]>
@whitneywhtsang whitneywhtsang mentioned this issue Aug 21, 2023
Closed
@whitneywhtsang
Copy link
Contributor Author

Remaining E2E test cases to handle:

  SYCL :: Plugin/enqueue-arg-order-image.cpp
  SYCL :: KernelFusion/work_group_barrier.cpp
  SYCL :: Plugin/interop-level-zero-image-ownership.cpp
  SYCL :: Plugin/interop-level-zero-image.cpp

@sommerlukas
Copy link
Contributor

Remaining E2E test cases to handle:

  SYCL :: Plugin/enqueue-arg-order-image.cpp
  SYCL :: KernelFusion/work_group_barrier.cpp
  SYCL :: Plugin/interop-level-zero-image-ownership.cpp
  SYCL :: Plugin/interop-level-zero-image.cpp

These are fixed with the patches in #11000 and #11016.

@whitneywhtsang
Copy link
Contributor Author

What's left to be done? Remove all typed pointer handling?

@sommerlukas
Copy link
Contributor

Yes, I think left to do include:

  • Remove patterns and other code dealing explicitly with the case of typed pointers
  • Remove the options from passes and cgeist
  • Remove test-cases explicitly testing typed pointer behavior

I was currently planning to do this only after a "cool-down" phase of maybe a couple of weeks, in case any regressions only appear later.

WDYT? Would you prefer to do this right away?

@whitneywhtsang
Copy link
Contributor Author

WDYT? Would you prefer to do this right away?

Let's wait for sycl branch to remove all typed pointer handling first, they are currently guarded by INTEL_SYCL_OPAQUEPOINTER_READY.

@whitneywhtsang
Copy link
Contributor Author

After #11160, all typed pointer handling in sycl branch are removed.

@sommerlukas
Copy link
Contributor

Opaque pointers are now the default and typed pointer support has been removed from SYCL-MLIR in #11181.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sommerlukas sommerlukas

@victor-eds victor-eds

@whitneywhtsang whitneywhtsang

@etiotto etiotto

Labels
enhancement New feature or request sycl-mlir Pull requests or issues for sycl-mlir branch
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@sommerlukas @victor-eds @whitneywhtsang @etiotto