Skip to content

build(cmake): make OptiX PTX available to build-tree tests#352

Open
plexoos wants to merge 3 commits into
mainfrom
ptx-in-build
Open

build(cmake): make OptiX PTX available to build-tree tests#352
plexoos wants to merge 3 commits into
mainfrom
ptx-in-build

Conversation

@plexoos
Copy link
Copy Markdown
Member

@plexoos plexoos commented Jun 5, 2026

Make generated OptiX PTX available from the build tree so uninstalled test and development workflows
can resolve CSGOptiX7.ptx.

Motivation

Some ctest targets and local build-tree runs instantiate the CSGOptiX pipeline, which requires the
generated CSGOptiX7.ptx file at runtime. Previously, runtime lookup only fell back to the install
libdir, so these workflows required either installing the project first or manually setting
CSGOptiX__optixpath.

This PR gives the build tree a stable PTX location and teaches runtime lookup to search it.

plexoos added 2 commits June 5, 2026 17:29
- Add a stable build-tree PTX output directory at `${PROJECT_BINARY_DIR}/ptx`
- Copy generated OptiX PTX files into that directory via a `gphox_ptx` target
Copilot AI review requested due to automatic review settings June 5, 2026 21:31
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fef177cff3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/config_path.h.in Outdated

#define GPHOX_CONFIG_SEARCH_PATHS ".:config:@GPHOX_INSTALL_FULL_DATADIR@/config"
#define GPHOX_PTX_DIR "@CMAKE_INSTALL_FULL_LIBDIR@"
#define GPHOX_PTX_SEARCH_PATHS "@GPHOX_BUILD_PTX_DIR@:@CMAKE_INSTALL_FULL_LIBDIR@"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Prefer installed PTX for installed binaries

Because this generated header is compiled into gphox for both build-tree and installed artifacts, the absolute @GPHOX_BUILD_PTX_DIR@ is still present after cmake --install. Config::PtxPath searches entries in macro order and returns the first existing file, so installed executables run on the build host will load ${build}/ptx/CSGOptiX7.ptx instead of the installed libdir copy; after a rebuild or multi-config build that can silently use stale or configuration-mismatched PTX. Consider using separate build/install-time paths or ensuring installed artifacts prefer only the install libdir.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes generated OptiX PTX discoverable from the build tree (in addition to the install tree), so uninstalled ctest/local workflows can resolve CSGOptiX7.ptx at runtime via Config::PtxPath.

Changes:

  • Introduces a build-tree PTX directory (${PROJECT_BINARY_DIR}/ptx) and adds it to the PTX search path list.
  • Copies the generated OptiX PTX into the build-tree PTX directory as part of building CSGOptiX.
  • Refactors colon-delimited search-path parsing into a shared helper used by both config and PTX lookup.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/config.cpp Adds generic search-path splitting and updates PTX resolution to search build + install candidates with improved error output.
src/config_path.h.in Replaces single PTX dir macro with a build+install PTX search path macro.
CSGOptiX/CMakeLists.txt Adds a custom target to copy the generated PTX into the build tree and wires it into the library build.
CMakeLists.txt Defines GPHOX_BUILD_PTX_DIR for use by build rules and configured headers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/config.cpp
Comment on lines +68 to +82
std::vector<std::string> SplitSearchPaths(std::string_view paths)
{
std::vector<std::string> search_paths;

size_t last = 0;
size_t next = 0;
while ((next = paths.find(':', last)) != std::string_view::npos)
{
search_paths.push_back(std::string{paths.substr(last, next - last)});
last = next + 1;
}

search_paths.push_back(std::string{paths.substr(last)});
return search_paths;
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is true for Windows-style drive-letter paths, but Simphony currently does not support Windows and the existing config search path convention is already Unix-style colon separation. I’m going to keep the current parser scoped to supported platforms for now.

@plexoos plexoos self-assigned this Jun 5, 2026
@plexoos plexoos added the build label Jun 5, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread src/config_path.h.in
Comment on lines 3 to +4
#define GPHOX_CONFIG_SEARCH_PATHS ".:config:@GPHOX_INSTALL_FULL_DATADIR@/config"
#define GPHOX_PTX_DIR "@CMAKE_INSTALL_FULL_LIBDIR@"
#define GPHOX_PTX_SEARCH_PATHS "@CMAKE_INSTALL_FULL_LIBDIR@:@GPHOX_BUILD_PTX_DIR@"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants