[BugFix][OpenCL] Guard QCOM perf hint behind USE_OPENCL_EXTN_QCOM to avoid undefined symbol on non-QCOM runtimes#18581
[BugFix][OpenCL] Guard QCOM perf hint behind USE_OPENCL_EXTN_QCOM to avoid undefined symbol on non-QCOM runtimes#18581ping-ee wants to merge 2 commits intoapache:mainfrom
Conversation
Summary of ChangesHello @ping-ee, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a build issue in the OpenCL runtime where a recent update to Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly identifies a linking issue with QCOM performance hints on non-QCOM OpenCL runtimes and proposes a fix by adding a preprocessor guard. The approach is sound, but there appears to be a typo in the macro name used in the implementation, which I've pointed out in a specific comment. Once that is addressed, this change should resolve the reported problem.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request addresses a linking issue with Qualcomm OpenCL performance hints that can occur on non-Qualcomm runtimes. The change correctly guards the usage of clSetPerfHintQCOM with the USE_OPENCL_EXTN_QCOM macro, in addition to the existing check for CL_CONTEXT_PERF_HINT_QCOM. This prevents undefined symbol errors when the OpenCL headers define the hint macro but the runtime library doesn't implement the function. The fix is clean, targeted, and aligns with how other QCOM-specific extensions are handled in the codebase. The change is correct and I have no further suggestions.
srkreddy1238
left a comment
There was a problem hiding this comment.
Accepted. Thanks for the fix.
|
Hi, I'm new to contributing to TVM. This PR was approved and seems ready to be merged, but the required Could a maintainer please help trigger or approve the Jenkins CI? |
|
@tvm-bot rerun |
|
Failed to re-run CI in https://github.com/apache/tvm/actions/runs/20279301153 Detailswith response |
|
The CI job didn’t trigger because the PR was created while CI was experiencing a disk space limit issue. @ping-ee, would you mind opening a new PR with the same changes? |
|
Closing this PR and opening a new one (#18589) per maintainer’s suggestion, since the Jenkins CI did not trigger due to a disk space issue. |
…avoid undefined symbol on non-QCOM runtimes (#18589) This PR is a re-open of #18581 The previous PR was created while Jenkins CI was experiencing a disk space issue and the CI job did not trigger. ## PR Description Recent OpenCL-Headers update (KhronosGroup/OpenCL-Headers#277 ) added QCOM perf-hint definitions (`CL_CONTEXT_PERF_HINT_QCOM`, `clSetPerfHintQCOM`) to `cl_ext.h`. These macros are now defined even on platforms whose OpenCL runtimes (e.g., PoCL, ICD loaders) do not implement the QCOM extension. TVM previously enabled the perf-hint code path solely based on the presence of `CL_CONTEXT_PERF_HINT_QCOM`, causing link errors such as: ``` undefined symbol: clSetPerfHintQCOM ``` This PR guards the QCOM perf-hint logic behind `USE_OPENCL_EXTN_QCOM`, matching the behavior of other QCOM-specific OpenCL paths (e.g., `SetNativePtr`). ## Effects Prevents accidental linking against unsupported QCOM symbols on non-QCOM runtimes. Keeps QCOM builds fully functional when `USE_OPENCL_EXTN_QCOM` is explicitly enabled. Aligns TVM’s extension handling across OpenCL code paths. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
PR Description
Recent OpenCL-Headers update (KhronosGroup/OpenCL-Headers#277
) added QCOM perf-hint definitions (
CL_CONTEXT_PERF_HINT_QCOM,clSetPerfHintQCOM) tocl_ext.h.These macros are now defined even on platforms whose OpenCL runtimes (e.g., PoCL, ICD loaders) do not implement the QCOM extension.
TVM previously enabled the perf-hint code path solely based on the presence of
CL_CONTEXT_PERF_HINT_QCOM, causing link errors such as:This PR guards the QCOM perf-hint logic behind
USE_OPENCL_EXTN_QCOM, matching the behavior of other QCOM-specific OpenCL paths (e.g.,SetNativePtr).Effects
USE_OPENCL_EXTN_QCOMis explicitly enabled.