Skip to content

Add PORTING.md: status, memory model, and roadmap for the GPU port - #142

Merged
rmrsk merged 2 commits into
devfrom
porting-overview
Aug 12, 2026
Merged

Add PORTING.md: status, memory model, and roadmap for the GPU port#142
rmrsk merged 2 commits into
devfrom
porting-overview

Conversation

@rmrsk

@rmrsk rmrsk commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Summary

Background

Issue #115 has been the only overview of the GPU port, and its tier checklist is stale: it
describes the first attempt (PRs #121-#129), none of which is on dev -- that history runs
straight from #120 to #130. Anyone picking up the port has had no single place to learn the memory
model it is built on, what is ported so far, or why the previous attempt was rewound.

Solution

Adds PORTING.md at the repository root as the authoritative status document, leaving #115 as the
record of how individual design decisions were argued out.

Its core is the memory model -- store an offset, not a pointer; the base is supplied by the
caller
-- and a post-mortem of the first attempt. That attempt did not fail on its evaluation
design; it failed on memory placement, which it treated as a final integration step rather than a
foundation:

  • TapeView<T> held raw pointers, so one view value could never be valid on both host and device;
    DeviceTape had to rebuild a second view with device addresses after upload.
  • DeviceTape hand-rolled a single-purpose pool (DeviceTapeAlign = 256, padUp(),
    paddedBytes(), uploadArray()) that nothing else could reuse. PoolBaseAlign is the same 256
    bytes today.
  • Upload enumerated every array by hand, mitigated by driving it off the X-macro registry.

All three are work created by designing host-first around std::vector and adding device residency
afterwards, which is why the restart landed the memory foundation first as #130.

The document also records what survived (the GPU portability layer, carried into #130 and extended
with the managed/pinned/mapped wrappers; the linear-SSA tape design), the current ported /
not-ported inventory, the roadmap, a recipe for porting a class, and how to actually run the device
tests locally.

Two roadmap decisions are captured explicitly:

  • The analytic-SDF trait refactor is separated from the tape. The formulas carry no
    memory-placement problem at all, so that step is independent of the memory work and can run in
    parallel with the BVH port instead of queuing behind it.
  • SignedDistanceFunction<T> stays for now. The first attempt deleted it as part of the trait
    refactor; that is a user-visible API break with no GPU motivation of its own and should be
    revisited on its own terms.

Side-effects

Documentation only -- no source, test or build changes, so no effect on the library, its API or its
test suite.

  • README.md gains one sentence pointing GPU-port contributors at PORTING.md.
  • Docs/Sphinx/source/Contributing.rst gains the same pointer, cross-referenced to the existing
    Memory model page.
  • REUSE.toml lists PORTING.md in the existing bulk-licensing block for top-level markdown.

Sphinx builds clean (0 warnings), and reuse, codespell and check-docs all pass.

Alternative solutions

  • Update issue GPU port #115's checklist in place. Rejected: a checklist buried in a long issue thread is
    not discoverable from the repository, and the issue is more useful left as the historical record
    of the decisions rather than rewritten to match current state.
  • A Sphinx page instead of a root markdown file. Rejected for now: the Sphinx set is user-facing
    documentation of what the library does, whereas this is contributor-facing status for work in
    progress, closer in kind to CLAUDE.md. It is linked from the Contributing page so it is still
    reachable from the user guide.

Reviewer checklist (to be completed by a human)

  • The test suite compiles and runs to completion without warnings or errors.
  • All relevant new features are documented in the user documentation (Sphinx).
  • This contribution does not break existing sections in the user documentation.
  • All relevant APIs are documented in the doxygen documentation.
  • Appropriate labels have been assigned to this PR.
  • New or revised proper licensing and copyright information is in place.
  • A PR review has been run using @claude review.
  • The continuous integration and testing hooks at GitHub run to completion.

rmrsk and others added 2 commits August 12, 2026 20:43
Issue #115 has been the only overview of the port, and its tier checklist is
stale -- it describes the first attempt (PRs #121-#129), none of which is on
dev, whose history runs straight from #120 to #130. This adds a repository-root
document that supersedes that checklist as the status reference, while leaving
#115 as the place the individual design decisions were argued out.

The bulk of it is the memory model the restart is built on -- store an offset,
not a pointer; the base is supplied by the caller -- and a post-mortem of why
the first attempt was rewound. That attempt did not fail on its evaluation
design; it failed on memory placement, which it treated as a final integration
step. Its TapeView held raw pointers, so the same value could never be valid on
both sides; DeviceTape hand-rolled a single-purpose pool (DeviceTapeAlign = 256,
padUp/paddedBytes/uploadArray) that nothing else could reuse; and every array
had to be enumerated by hand at upload. All three are work created by designing
host-first around std::vector and bolting device residency on afterwards, which
is why the restart landed the memory foundation first, as PR #130.

Also records what survived (the GPU portability layer, carried into #130; the
linear-SSA tape design), the current ported/not-ported inventory, the roadmap,
a recipe for porting a class, and how to actually run the device tests -- CI
compiles both backends but has no GPU, so a green GPU lane means "it compiles"
and nothing more.

The roadmap deliberately separates the analytic-SDF trait refactor from the
tape: the formulas carry no memory-placement problem at all, so that step is
independent of the memory work and can run in parallel with the BVH port,
rather than queuing behind it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first attempt's trait refactor deleted SignedDistanceFunction<T> outright
and collapsed everything onto ImplicitFunction<T, Op> + bool m_sdf. That is a
user-visible API break with no GPU motivation of its own, so it should not ride
along with the trait step; note it explicitly so the next person doing that work
does not repeat the removal by default.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rmrsk
rmrsk marked this pull request as ready for review August 12, 2026 18:46
@rmrsk

rmrsk commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

@claude review

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

Claude finished @rmrsk's task in 1m 22s —— View job


Review complete ✅

  • Gather context (PR diff, changed files)
  • Read PORTING.md in full
  • Check README.md / Contributing.rst / REUSE.toml edits
  • Cross-check claims against actual repo state
  • Post review feedback

Docs-only PR, and it's in good shape. I cross-checked the technical claims against the actual source tree rather than just reading prose:

  • PoolBaseAlign = 256 (Source/EBGeometry_MemoryResource.hpp:48) matches the "same 256 bytes today" claim.
  • Pool::mirror(const Pool&, MemoryResource&) (Source/EBGeometry_Pool.hpp:186) and deviceMemoryResource() (Source/EBGeometry_MemoryResource.hpp:476) exist with the signatures used in the PORTING.md code sample, and DCEL::MeshT::boundView() (Source/EBGeometry_DCEL_MeshImplem.hpp:50) matches too.
  • The EBGeometry_GPUDeviceTests target and gpu-device CTest label are real (Tests/CMakeLists.txt:101-129), and [gpu]-tagged cases exist in TestVec, TestBoundingVolumes, TestPool, TestPointAoSoA, TestTriangleSoA, TestDCEL — matching the "What is ported" table.
  • cuda/hip CMake presets exist in CMakePresets.json as described.
  • :ref:Chap:MemoryModel`` (added in Contributing.rst) resolves to a real label in `Docs/Sphinx/source/MemoryModel.rst`.
  • No literalinclude introduced; PORTING.md is correctly added to the bulk-licensing block in REUSE.toml; README.md/Contributing.rst pointers read cleanly.
  • "Status as of 2026-08-12, dev at fae8eeb" matches origin/dev's current HEAD.

Nothing to flag — no bugs to find in a status/planning document, and every concrete API/file claim I checked matches the current dev state. Two small, non-blocking observations, neither worth holding up the merge:

  1. The "What is ported" table (PR column: GPU port foundation (S0): POD memory layer — MemoryResource + Pool + PODVector #130DCEL: rewrite MeshT to drop its owned Pool pointer for device-mirror safety #140) cites specific historical PR numbers I couldn't independently re-verify against GitHub's PR history from this environment (no gh pr list access here) — worth a quick self-check by the author before merge, since a stale PR number would be the one way this doc could silently drift immediately after being written.
  2. Roadmap step 2 mentions an open decision on device stack depth ([256] vs 64) — purely a forward-looking note, not something this PR needs to resolve.

This is exactly the kind of document the project's CLAUDE.md post-mortem guidance calls for: it explains why the first port attempt failed (memory placement bolted on after the fact) rather than just what changed, which will save real time for whoever picks up the BVH port next.

@rmrsk
rmrsk merged commit 7a84e08 into dev Aug 12, 2026
44 of 62 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.

1 participant