Skip to content
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

[OpaquePointers] Fix translation of EnqueueKernel and block functions #1857

Merged
merged 2 commits into from Mar 23, 2023

Conversation

dwoodwor-intel
Copy link
Contributor

@dwoodwor-intel dwoodwor-intel commented Feb 24, 2023

This specifies the right parameter types for EnqueueKernel and for block
functions (with mangled names of the form {__,}block_invoke{,}{_kernel,}),
so that SPIR-V code is generated for these as expected. This fixes a
crash in the Intel OpenCL CPU backend, and likely other SPIR-V
consumers.

@dwoodwor-intel
Copy link
Contributor Author

This patch is ready for review, but depends on #1799 (which also depends on D141008). I am seeing some test failures which don't seem to be specific to this change; they probably need to be fixed in #1799 when that is updated to be merged:

Failed Tests (2):
  LLVM_SPIRV :: extensions/INTEL/SPV_INTEL_device_side_avc_motion_esimation/subgroup_avc_intel_vme_image.cl
  LLVM_SPIRV :: image-unoptimized.cl

********************
Unexpectedly Passed Tests (1):
  LLVM_SPIRV :: transcoding/pipe_builtins.cl

Copy link
Contributor

@MrSidims MrSidims 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!

This specifies the right parameter types for EnqueueKernel and for block
functions (with mangled names of the form {__*,}_block_invoke{_*,}{_kernel,}),
so that SPIR-V code is generated for these as expected. This fixes a
crash in the Intel OpenCL CPU backend, and likely other SPIR-V
consumers.
@dwoodwor-intel dwoodwor-intel marked this pull request as ready for review March 17, 2023 17:45
@dwoodwor-intel
Copy link
Contributor Author

I've rebased this on the current main branch now that the relevant fixes (and the test fix yesterday for #1885) are there. @jcranmer-intel also pointed out a few more block-related tests that need to be updated for opaque pointers, so I've updated those here too and fixed some bugs in my first implementation of the block part of this.

@MrSidims
Copy link
Contributor

Restarting CI with new head

@MrSidims MrSidims closed this Mar 21, 2023
@MrSidims MrSidims reopened this Mar 21, 2023
@MrSidims MrSidims requested a review from svenvh March 22, 2023 09:08
@MrSidims MrSidims merged commit 9a1cce0 into KhronosGroup:main Mar 23, 2023
8 checks passed
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.

None yet

3 participants