Skip to content

Update urProgramGetFunctionPointer test #2522

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 1 commit into from
Jan 13, 2025

Conversation

RossBrunton
Copy link
Contributor

The spec says that implementations may return
UR_RESULT_ERROR_FUNCTION_ADDRESS_NOT_AVAILABLE for this function, but
that wasn't handled by this test.

@RossBrunton RossBrunton requested a review from a team as a code owner January 6, 2025 15:34
@github-actions github-actions bot added the conformance Conformance test suite issues. label Jan 6, 2025
@aarongreig
Copy link
Contributor

This is a valid return but I'm not sure it's necessarily what we're expecting under the circumstances set up by the test for all platforms, I'm a bit worried a genuine error condition could go unnoticed down the line with this change. What platform were you seeing the error on?

@RossBrunton
Copy link
Contributor Author

This is a valid return but I'm not sure it's necessarily what we're expecting under the circumstances set up by the test for all platforms, I'm a bit worried a genuine error condition could go unnoticed down the line with this change. What platform were you seeing the error on?

I'm seeing it on my local system, which identifies as:

[opencl:gpu][opencl:0] Intel(R) OpenCL Graphics, Intel(R) UHD Graphics 770 OpenCL 3.0 NEO  [24.39.31294.12]

My reading of the spec is that implementations are allowed to return this value if they can't get the address of the function for some internal technical reason. Which basically means that, from the point of view of UR, implementations may (at their discretion) return UR_RESULT_ERROR_FUNCTION_ADDRESS_NOT_AVAILABLE provided the function is present.

@aarongreig
Copy link
Contributor

Ah, looks like this is one of the CL extensions with no documentation anywhere. Just to satisfy my curiosity, could you try creating a kernel for the function before trying to retrieve the function pointer? I'm wondering if maybe we aren't using the entry point right. Otherwise I guess we can just accept that return as tantamount to "not supported"

@RossBrunton
Copy link
Contributor Author

Ah, looks like this is one of the CL extensions with no documentation anywhere. Just to satisfy my curiosity, could you try creating a kernel for the function before trying to retrieve the function pointer? I'm wondering if maybe we aren't using the entry point right. Otherwise I guess we can just accept that return as tantamount to "not supported"

    ur_kernel_handle_t kern;
    urKernelCreate(program, function_name.data(), &kern);

Adding this doesn't cause the function to return a function pointer on my device.

@RossBrunton RossBrunton requested review from a team as code owners January 10, 2025 17:18
@github-actions github-actions bot added specification Changes or additions to the specification hip HIP adapter specific issues labels Jan 10, 2025
@RossBrunton RossBrunton marked this pull request as draft January 10, 2025 17:21
The spec says that implementations may return
`UR_RESULT_ERROR_FUNCTION_ADDRESS_NOT_AVAILABLE` for this function, but
that wasn't handled by this test.
@RossBrunton RossBrunton removed specification Changes or additions to the specification hip HIP adapter specific issues labels Jan 13, 2025
@RossBrunton RossBrunton removed request for a team, frasercrmck and Bensuo January 13, 2025 11:11
@RossBrunton RossBrunton marked this pull request as ready for review January 13, 2025 11:33
@RossBrunton RossBrunton merged commit 0125279 into oneapi-src:main Jan 13, 2025
73 checks passed
@RossBrunton RossBrunton deleted the ross/testchecks branch January 16, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conformance Conformance test suite issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants