Skip to content

[REFACTOR] Move source_utils.h into runtime/opencl#19456

Merged
spectrometerHBH merged 2 commits into
apache:mainfrom
tqchen:move-source-utils-to-opencl
Apr 27, 2026
Merged

[REFACTOR] Move source_utils.h into runtime/opencl#19456
spectrometerHBH merged 2 commits into
apache:mainfrom
tqchen:move-source-utils-to-opencl

Conversation

@tqchen

@tqchen tqchen commented Apr 27, 2026

Copy link
Copy Markdown
Member

Move source_utils.h header into runtime/opencl directory with SplitKernels inlined. Removes the standalone source_utils.cc file and updates 2 include paths to reflect the new location.

Test plan

  • Compilation of modified translation units verified
  • Full CI suite validates broader impacts

`src/runtime/source_utils.h` is only used by OpenCL backend code.
Move it into the opencl subdir, inline `SplitKernels` (single
short function), and delete the now-empty `.cc`.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request moves source_utils.h from the general runtime directory to the OpenCL-specific directory, updates include paths in OpenCL modules, and converts the SplitKernels function into an inline definition. Feedback was provided to optimize the SplitKernels function by passing string parameters by constant reference to prevent redundant memory allocations.

Comment thread src/runtime/opencl/source_utils.h Outdated
The header-only inline form copies the OpenCL source string on
every call; const reference avoids the copy. The function only
reads the parameters.
@tqchen

tqchen commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

Done: changed to const std::string& to avoid copying the source and delimiter strings on every call. The function only reads the parameters, so the reference form is safe.

@spectrometerHBH spectrometerHBH merged commit f42a279 into apache:main Apr 27, 2026
9 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.

2 participants