Skip to content

Add native AMD HIP/ROCm Tensor operators - #3809

Merged
MrNeRF merged 17 commits into
MrNeRF:devfrom
francescofugazzi:feat/hip-native-pointwise
Oct 26, 2026
Merged

MrNeRF merged 17 commits into
MrNeRF:devfrom
francescofugazzi:feat/hip-native-pointwise

Conversation

@francescofugazzi

@francescofugazzi francescofugazzi commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Purpose

This is the fourth part of the AMD HIP/ROCm work originally proposed in #3759, following the shared Tensor contracts (#3776), native runtime foundation (#3781), and owned asynchronous transfers (#3782).

It adds native Tensor operators behind the private HIP runtime boundary. The implementations use context-owned buffers and streams, validate descriptors before launching work, and report errors through the existing C ABI.

HIP is still not selectable in the application, and this PR does not enable HIP training. The public Tensor adapter, backend registration, image preparation, viewer integration and distribution remain separate work. No CUDA, Metal or Vulkan implementation or trainer is changed.

Native operations

  • Pointwise operations: the 68-operation FP32 catalog and the 17 reachable FP16 operations, byte predicate results, four FP32 scalar-left arithmetic operations, FP32 arange, device copy and byte fill. Generated dispatch includes the shared functor header directly through HIP guards; it does not maintain a copied functor implementation.
  • Typed storage and layouts: 24 supported non-identity numeric conversion pairs, matching the CUDA conversion matrix; integer/boolean operations, broadcasting, strided copy/scatter/fill and batches of layout copies. The README specifies supported dtype combinations.
  • Reductions: global and axis reductions, indexed extrema, and fused FP32 expression chains/reductions. Integer operations and comparisons preserve Int64 precision rather than passing through floating-point values.
  • Indexing and selection: gather, index copy/add/fill, masks, stable selection/nonzero and conditional selection, with operation-specific dtype validation.
  • Scans and sorting: cumulative sums and stable FP32 sorting with Int64 indices. Sorting preserves original NaN payloads and signed-zero bits while treating equivalent keys stably.
  • Random generation: seeded uniform, normal and Bernoulli FP32 draws, plus Int32 integer draws.
  • Matrices: FP32 batched matrix multiplication, optional transposed right-hand input and bias/ReLU, and pairwise distances with supported norms.

The developer README documents the storage subsets accepted by each family. Unsupported combinations fail explicitly; no implicit CPU fallback is introduced. Floating reductions may differ in rounding order between devices, and reproducible seeds do not imply identical RNG sequences across different backends.

Ownership, errors and edge cases

Views retain the context-owned buffer handle and byte offset. Operations validate ownership, ranges, alignment, layout spans, shape compatibility and their alias restrictions. Descriptor and range validation precedes submission. Checked indexing validates each lane in the kernel and skips out-of-range accesses; valid lanes may still be written, so this is not transactional rollback. Batched layout-copy descriptors are all validated before any copy is submitted. Exact in-place operations are accepted only where their contract permits them.

Reduction, scan and sort workspaces belong to the context. Growth drains previous users and releases disposable storage before allocating its replacement. Failed release retains ownership for retry; trim and context destruction release all workspace categories. Host scalar readback drains the stream before a blocking copy, so a failed drain cannot leave a queued write into expired stack storage.

The private ABI is revision 5. All reduction entry points use LfsHipReduceOp, including typed axis reductions. Loaders check the exact revision before binding exports; revisions 1 through 4 are rejected. The readback correction changes opaque internal state and error propagation, not exported signatures, so it does not require another revision.

The local correctness review also added regressions for capped launch grids above 1,048,576 elements and shifted byte-predicate aliases. Those cases failed against the preceding implementation and pass after the corresponding corrections.

Maintained tests and CI

The SDK-only tests compare native results with independent small CPU/scalar references. They cover conversions, large integers, NaN/Inf/signed zero, stable ordering, empty axes, noncontiguous layouts, partial launch blocks, invalid descriptors and output preservation. Matrix-distance tests cover short and tiled paths and multiple norms; indexed addition tests include duplicate destinations.

The separate instrumented module exercises allocation/free and stream-drain failures, workspace growth/trim, context cleanup retries and balanced device/pinned/event resource counters. Fault-injection entry points remain absent from the production module.

CPU-only module checks bind the native exports without querying a GPU. Generator regressions run without an SDK and join the existing Python fast suite. The path-gated HIP compile job inherited from #3782 additionally watches the shared pointwise catalog, functors and generator tests, so changes to those inputs cannot silently bypass native compilation. Workflow triggers and non-HIP jobs remain unchanged.

Deferred errors and readback snapshots

Index and masked-scatter cardinality errors surface at context synchronization, blocking host readback, or staged readback wait/poll. A staged readback captures preceding execution errors at its own queue position before its completion event. Later operations cannot invalidate an earlier snapshot. Packed appends accumulate their captured errors, including across context-error consumption/reset. Wait/poll report a failed snapshot before exposing bytes or successful readiness; a drained null poll discards it while returning its error. Context synchronization consumes its own pending error independently and cannot validate an already failed snapshot.

Ordinary readback and ordering markers need no additional kernel: the error flag is copied on the producer stream into slot-owned pinned storage. Packed accumulation uses a slot-owned four-byte device flag. Both remain owned across capture, synchronization and cleanup failures. Shared fill, affine and arange kernels now each have one definition.

Latest local validation

At 9abcf088f:

  • Four new regression methods were run against the previous b28947e0c module and reproduced the missing readback error (five failed assertions including the marker variants). They pass with the correction.
  • Windows SDK 10.1.0, gfx1150: normal/instrumented modules rebuilt; 9/9 CTest groups passed. Additional focused runs passed after the final direct-poll and masked-scatter regressions.
  • Regressions cover unchanged destination bytes and ready=0 on error, earlier-valid/later-invalid snapshots, context-error consumption, packed appends, empty readbacks, ordering markers, failed-result discard/reuse, and allocation/copy/event/free failure recovery.
  • WSL/Linux, SDK 10.1.0: all 26 compressed targets compiled; 5/5 CPU CTest groups passed, without Linux GPU execution.
  • 71 CPU Python tests passed, including the 26 configuration/generator/filter tests and 45 error-debt tool tests. The actual test output is retained locally.
  • clang-format 19 across the PR's 23 C/C++/HIP files, conversion checks on all three HIP translation units, whitespace and error-debt checks passed.
  • The CI path filter now includes pointwise_lowering.hpp, an input read by the pointwise generator.

The preceding b28947e0c revision also compiled the 26-target compressed module locally in 4m23.69s, compared with 9m28.477s for the earlier configuration below. That combined reduced instantiations, three translation units and --offload-jobs=2; it is a compilation comparison, not a training-performance claim. The final readback revision was rebuilt for all targets without a new controlled timing comparison.

Earlier review validation at df889f4 (retained)

At df889f403, including a7c7c11bb:

  • Windows, SDK 10.1.0, gfx1150: normal and fault-injection modules rebuilt; 9/9 CTest groups passed. New independent regressions cover fractional byte/Bool scalar casts, common reduction opcodes, sparse selection workspace, stable segmented sorting, payload preservation, integer unary overflow/precision and reduction dispatch thresholds.
  • 432 scalar/axis/fused-chain reduction outputs compared before and after the consolidation: zero bitwise differences in those cases. This is a bounded operator comparison, not a claim of universal bitwise equality.
  • Three focused new tests fail against the pre-correction module and pass against the corrected module. An actual ABI 3 module is rejected before binding exports.
  • Ubuntu 24.04 in WSL, SDK 10.1.0: the final module built for all 26 declared targets with the exact compressed CI configuration, in 9m28.477s; 5/5 CPU CTest groups passed. This does not claim GPU execution on those targets.
  • 19 CPU Python configuration/generator checks passed. clang-format 19, host/device -Wconversion with no warnings, whitespace, error-debt baseline and actionlint passed.
  • The gfx1150 Windows DLL decreased from 9,296,896 to 6,946,304 bytes. The final compressed all-target Linux module is 4,777,968 bytes. These are different platforms/configurations and should not be compared as compression ratios.

The HIP-only CI job timeout is now 60 minutes. DeviceSelect is instantiated by four storage widths, and ascending/descending sorting share the same radix-sort direction. No non-HIP job is changed.

Earlier validation (retained for reference)

After rebasing onto dev at a64dfee72, including the merged #3782 follow-up:

  • Windows, SDK 10.1.0, gfx1150: normal and instrumented modules rebuilt; 9/9 CTest groups passed, including 21 operator tests, 4 workspace-recovery tests and the retained pointwise/transfer contracts.
  • 68 Python/source checks passed: 26 CI-filter/configuration/generator tests and 42 Windows build/path contracts; source preflight passed.
  • Ubuntu 24.04 in WSL, SDK 10.1.0: the final branch compiled for gfx1150 and gfx942; 5/5 CPU CTest groups passed with GPU visibility disabled.
  • An earlier operator snapshot also compiled the compressed module for all 26 declared targets. The final rebased branch was rechecked on the two targets above; this is not a claim that all 26 targets were rebuilt or exercised at the final head.
  • clang-format 19 across the native directory and fault header, whitespace, error-debt baseline and actionlint passed.
  • The rebase comparison verified unchanged native kernels/generator against the previously tested snapshot. ABI versioning, module expectations and CI dependency selection were adapted to the merged transfer changes.

These checks qualify the private native operator boundary. They do not establish full application integration, training quality/performance, physical device-loss recovery, Linux GPU execution, or runtime correctness on unavailable hardware, including wave64 devices. No new macOS execution is claimed for the final rebase; HIP compilation remains unsupported there.

Only implementation, maintained tests and build/capability documentation are committed. Local reports, benchmark experiments and datasets are excluded.

@francescofugazzi francescofugazzi added the dev branch Applies to the dev branch only label Oct 25, 2026 — with ChatGPT Codex Connector

@MrNeRF MrNeRF left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Built it with the pinned SDK 10.1.0:

  • Builds: gfx1150 and gfx942 build in under a minute each. The full 26-target compressed module builds with the CI job's exact flags.
  • Tests: all CPU CTest groups pass (5/5 for each build), and the new Python tests pass.
  • ABI revision: revision 3 rejects real revision 1 and 2 modules, not just the mock.
  • Contracts: from reading, most operators match the CUDA backend: sort NaN/±0 ordering and stability, argmin/argmax, conversions, index clamping, index_add duplicates, cdist p=0/inf and empty axes.
  • CI filter: it works on real history and leaves the other jobs alone.
  1. The 26-target compile will likely exceed the job's timeout.
    • It took 906 s here, against 26 s for #3782. Device compile and link run per target inside one clang call, so --parallel doesn't help.
    • #3782's step ran about 1.6x slower on GitHub than here. That puts this one at about 25 minutes, against the job's timeout-minutes: 20 (ubuntu.yml:47).
    • The PR's own run was cancelled before it started, so this hasn't been tried on CI.
    • Could you cut the instantiations (see 5), parallelise the offload compile/link, or raise the timeout?
  2. Byte predicates compare the scalar as a float, float(a) < scalar (tensor_ops_runtime.hpp:822–831). CUDA casts the scalar to the element type instead (scalar_right_op, tensor_functors.hpp:885–888).
    • For a UInt8 tensor, x=2 < 2.5 is false on CUDA and true here.
    • x=0 == 0.5 is true on CUDA and false here.
    • predicate_typed already does the cast for I32/I64/U32. Routing UInt8/Bool through it fixes this and drops boolean_pointwise (~55 lines).
  3. Reduction op codes mean two different things within the same ABI.
    • lfs_hip_reduce_typed_layout uses the core ReduceOp numbering, 2=Max and 3=Min (integer_reduce_runtime.hpp:61, 111–112).
    • lfs_hip_reduce_f32, reduce_layout_f32 and chain_reduce_f32 use LfsHipReduceOp, where MIN=2 and MAX=3 (runtime_api.h:149–152).
    • integer_reduce_api.h names no enum, so an adapter passing LFS_HIP_REDUCE_MAX to the typed reduction gets min.
    • Could both use one enum?
  4. masked_select always stages its result in a full-size workspace that stays allocated.
    • select_stable only writes directly when capacity >= count (tensor_ops_runtime.hpp:768–781), but the frontend passes the exact count (tensor_masking_ops.cpp:273–295).
    • A 100M-element float masked_select therefore keeps ~400 MB until trim_workspace. CUDA writes straight into the result.
  5. Size.
    • There are ~3,270 implementation lines. A single-target module grew from 61 KB to 11 MB with 3,045 device functions. hipcub/rocprim make up 56% of that: 144 segmented radix sort kernels, and 1,040 DeviceSelect kernels from 11 type combinations.
    • About 300–350 lines repeat logic that already exists:
      • integer_math_native duplicates the shared div/mod/pow/min/max/abs/neg/relu/sign functors that you already compile in, and clamp_i32 duplicates clamp_range_op.
      • The four block-reduction kernel families, with copied parts-splitting and kept/reduced lambdas, could be one.
      • 17 overlap lambdas and 6 copies of the lazy workspace allocation could each be one helper.
      • lfs_hip_broadcast_f32 and lfs_hip_fill_strided_f32 are aliases of broadcast_typed and fill_layout_bits.
    • A single DeviceSelect::Flagged per element width, instead of 11 type combinations, would also shrink the binary and help with 1.

Smaller things:

  • ReLU(-0) gives -0 through the generated functor (prepare_pointwise.py:26–30). It gives +0 on CUDA and in your own GEMM bias+ReLU (gemm_runtime.hpp:19, 57). Worth picking one.
  • There are 44 narrowing warnings under -Wconversion, for example dim3 in gemm_runtime.hpp:94–111 and tensor_ops_runtime.hpp:1507/1509. They're harmless given the validation but easy to clean up.

Copy link
Copy Markdown
Collaborator Author

Addressed the review in a7c7c11bb and df889f403.

  1. Compile cost / timeout: the HIP-only job now allows 60 minutes. DeviceSelect uses one instantiation per 1/2/4/8-byte storage width, and both sort directions share ascending radix sort through canonical transformed keys. The final 26-target compressed Linux module builds with the exact CI configuration.
  2. Byte predicates: the byte API delegates to typed predicates, so the scalar is converted to the input dtype before comparison. Bool retains its own scalar conversion semantics. Fractional-scalar regressions cover the reported cases.
  3. Reduction opcodes: every entry point uses LfsHipReduceOp; typed reductions no longer accept Core enum ordinals. This advances the private ABI to 4. An actual ABI 3 DLL is rejected before export binding.
  4. masked_select workspace: exact-sized output capacity is validated with a counter-only preflight before writing directly to the result. The full-size staged result is gone. Tests check sparse workspace usage, insufficient-capacity output preservation and raw selected bits.
  5. Consolidation: the four block-reduction families now use one kernel template with separate input readers, accumulator policies and output stores. Short reductions share a serial fast path. Parts selection, layout decomposition and lazy workspace allocation are shared; the previous axis/chain final-pass accumulation orders are retained. Local overlap-validation lambdas were replaced by common checked-range helpers. Integer arithmetic reuses the shared functors, retaining unsigned wraparound and the signed division/modulo overflow guards. Clamp uses clamp_range_op; floating convenience exports are thin forwarding wrappers without duplicate GPU implementation.

The two smaller comments are addressed: HIP device ReLU(-0) now produces +0 consistently with fused GEMM/ReLU, and host/device -Wconversion reports no warnings.

Validation at the updated head

  • Windows gfx1150, SDK 10.1.0: 9/9 CTest groups passed, including normal and instrumented modules.
  • 432 scalar/axis/fused-chain reduction outputs before/after consolidation: zero bitwise differences in the checked cases.
  • Three focused regressions fail on the previous module and pass on the corrected module.
  • Linux WSL, all 26 declared targets, compressed exact CI configuration: build 9m28.477s, CPU CTest 5/5.
  • CPU Python checks 19/19; clang-format, error-debt, actionlint and whitespace checks pass.

The PR description now records these results while retaining the earlier validation as historical context. Linux checks remain compilation/CPU coverage, not GPU execution on unavailable devices. This still does not enable the application Tensor adapter or training.

@MrNeRF MrNeRF left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Rebuilt it at df889f4 with SDK 10.1.0: gfx1150, gfx942, and the 26-target compressed module with the CI flags. This box has no AMD GPU, so the GPU test groups didn't run.

What's fixed:

  • CI time: the job allows 60 minutes. The 26-target build took 829 s here (~22 min at GitHub's speed). The module shrank from 6.1 to 4.8 MB, and gfx1150 from 11.0 to 8.0 MB.
  • Reductions: one enum. The real ABI 3 module is rejected with "expected 4, got 3" before any export is bound.
  • masked_select: no full-size staging buffer anymore.
  • ReLU(-0): gives +0 everywhere. -Wconversion drops from 48 warnings to 0 (host and device).
  • Consolidation: shared integer functors, clamp_range_op, one block-reduction kernel and one workspace helper. The accumulation order is unchanged in every path, from reading.
  • Tests: CPU CTest 5/5 on all three builds, Python 26/26.

A correction to my round-1 point 2: it was wrong. Through the Tensor API, a UInt8 tensor compared with a float scalar is promoted to Float32 before any backend sees it (promote_dtypes in the scalar comparison ops). So on CUDA, 2 < 2.5 is true, and the backend's cast is never reached. Your change is harmless; sorry for the noise.

What I'd still change:

  1. Bool now has two rules.
    • predicate_typed with Bool converts the scalar with bool(scalar), so 0.5 becomes true (predicate_runtime.hpp:62–70).
    • lfs_hip_boolean_pointwise passes Bool data as U8 (tensor_ops_runtime.hpp:924), so 0.5 becomes 0.
    • The test pins the first rule, but #3759's adapter used boolean_pointwise.
    • Deleting boolean_pointwise leaves one rule.
  2. The selection kernels grew 2–3.3x. DeviceSelect went from 177 to 275 KB per target.
    • Values are read through a counting iterator plus memcpy, and the mask through a per-element switch over six dtypes, although value selection always has a UInt8 mask (tensor_ops_runtime.hpp:806–896).
    • Plain pointers per width, plus one generic instantiation for nonzero positions only, would fix it.
    • selection_count (:832–847) is the same kernel as count_nonzero_kernel (:119–140).
  3. Extra passes and host syncs that CUDA doesn't do. The last one matters most for training loops.
    • masked_select now costs 3 mask passes and 3 syncs, against 2 and 1 on CUDA. The frontend already counted and passes the exact count, and CUDA trusts it (cuda_ops_index_mask.cpp:288–301).
    • masked_scatter rescans and syncs to recheck selected_count (:1139–1155).
    • check_indices runs a validation kernel and a blocking read before every checked gather, index_select, index_fill, index_add and index_cast (:619–631). CUDA records index faults asynchronously instead.
  4. What can go. Nothing below is reachable from the Tensor API, and CUDA has no counterpart either:
    • Exports without a caller, or that only forward:
      • broadcast_f32, fill_strided_f32 and boolean_pointwise;
      • scalar_left_typed: nothing in src/ builds a scalar-left program, and it is 28 kernels per target;
      • where_scalar;
      • the workspace_growth_bytes out-parameter on arg-extreme. memory_info already reports it.
    • Instantiations:
      • the generator emits all 68 ops for Float16, but the Tensor API reaches 17 of them (prepare_pointwise.py:74–78);
      • 42 conversion pairs, where CUDA serves 24.
      • Together that's about 150 kernels per target.
    • Duplicates:
      • strided copy and strided scatter are special cases of copy_layouts;
      • validate_indices and the inline check in index_cast repeat check_indices;
      • index_fill_kernel is index_update_kernel with a constant source;
      • the checked shape product is written 4 times;
      • 12 hand-written overlap checks remain next to the new helpers;
      • the comparison switch in integral_broadcast_runtime.hpp:15–28 repeats predicate_runtime.hpp:11–30.
    • Generated code: hip_pointwise_functors.hpp is a 1,592-line text-patched copy of the shared functors. A device-guard macro in the shared header would let HIP include it directly. The generator's SCALARS/COMPOSITES tables also restate pointwise_lowering.hpp.
    • README: about 35 of the 95 lines narrate code or review history: the block kernel, the enum, DeviceSelect, the generator and the GPU tests. The support table and the contract paragraphs are what readers need.
    • Tests: the two sort tests have the same body. Five reduction tests and five byte-predicate tests overlap heavily.
    • Total: that's roughly 250 implementation lines, 70 test lines and 35 README lines.
  5. CI time:
    • The 60-minute limit hides that everything is one compilation unit: runtime.hip includes the nine *_runtime.hpp fragments, so the 26-target build is one serial clang call of ~14 minutes, and any edit rebuilds all of it.
    • Adding --offload-jobs=2 cut it from 829 s to 600 s here (one CMake line).
    • Splitting the runtime into 3–4 .hip files would fix it properly.

Smaller things:

  • The ABI style is mixed.
    • Some calls take versioned descriptors, while gather and index_update take 11–12 positional parameters.
    • LfsHipPredicate doubles as the integer-arithmetic descriptor, and its reserved field means "scalar on the left".
    • LFS_HIP_REDUCTION_WORKSPACE_FLOATS is part of the ABI.
  • CI: the HIP compile job of run 40216155252 has been queued since 18:33 UTC and hasn't started yet.

Copy link
Copy Markdown
Collaborator Author

Follow-up to the latest review: pushed the revisions through b28947e0c.

Operator surface and kernel instantiations

  • Removed the duplicate Boolean entry point; Boolean/UInt8 operations now use the typed predicate path.
  • Replaced the overloaded predicate descriptor for integer arithmetic with a dedicated descriptor and renamed the predicate's reserved field to scalar_left.
  • Removed redundant broadcast/fill aliases and the reduction workspace constant/output parameter from the ABI.
  • Reduced FP16 pointwise instantiations from 68 to 17 reachable operations and restricted numeric conversion kernels to the 24 supported non-identity conversion pairs.
  • The pointwise generator now includes the shared functor header and reads lowering aliases from the shared definitions instead of maintaining a copied functor implementation and duplicate operation tables.

I retained where_scalar: it has actual callers in the Tensor adapter and a corresponding CUDA implementation. Removing its functionality would require replacing those calls, potentially by materializing a temporary scalar-filled Tensor. I also retained the FP32 scalar-left arithmetic used by the adapter and supported by the CUDA counterparts, but reduced its kernel instantiations from 28 to 4: Add, Sub, Mul and Div. Subtraction and division require preserving operand order. This is a deliberate exception to deleting these entry points wholesale: the used functionality remains, while unnecessary specializations have been removed. Consolidating their ABI entry points further would be a separate interface refactor, rather than removal of unused functionality.

Selection and indexing

  • Value selection now uses plain typed pointers by element width; the generic counting/flag iterator remains only for position selection.
  • Unified the nonzero count implementation and added a versioned selection descriptor. The precounted path uses the frontend's existing count without another host count read; it requires the same unchanged mask and a correct count.
  • Masked scatter validates cardinality on the device before writing output, eliminating the host prefix/flag reads.
  • Removed the separate blocking index-validation passes. Gather, index-select, index-update/add/fill and index conversion validate accesses in their kernels and report invalid indices at an explicit synchronization/read boundary. Invalid lanes are skipped; this is not a transactional rollback of valid lanes.
  • Added regression and fault-injection coverage for allocation/free failures, synchronization failures, error consumption and recovery.

The small asynchronous error flag uses owned pinned host staging and a copy queued on the producer stream. This follows a reproducible SDK/driver observation on the tested Windows system where a blocking four-byte flag copy did not observe the value seen by the queued copy. The completed fence leaves no reset queued behind it: reset is deferred until the next checked submission. A fault-injection test verifies that distinction.

ABI and build organization

  • Native interface ABI is now 5, reflecting incompatible descriptor/signature changes. Internal refactors alone do not require another increment.
  • Split compilation into runtime, pointwise and operator translation units, with depfiles and incremental dependency tracking.
  • Added configurable offload compilation parallelism, defaulting to two jobs.
  • Updated the public capability and synchronization documentation.

Validation

  • Windows standalone native module: 9/9 CTest groups passed, including the final synchronization/fault-injection changes.
  • The local Tensor adapter was aligned separately: HIP 35/35 and Vulkan 35/35 public contract tests passed; CPU had 29 passed and 6 explicit GPU-only skips.
  • 80/80 CPU Python tests passed in the adapter validation, along with HIP-only/CUDA+HIP and HIP-disabled syntax checks.
  • WSL/Linux: native compilation for all 26 configured architectures and 5/5 CPU tests passed. The separate extended adapter also compiled for gfx1150 and gfx942 with 5/5 CPU tests. No Linux GPU execution is claimed.
  • clang-format 19, whitespace and local debt checks passed. The three native HIP translation units also passed the conversion-warning checks.
  • Host, CUDA-host and CUDA-device preprocessing comparisons for the shared functor header were unchanged. No fresh CUDA GPU or macOS runtime test is claimed for this revision.

For the measured Linux compilation configuration, the 26-target build decreased from 9m28.477s to 4m23.69s. This combines the reduced instantiations, translation-unit split and offload parallelism; it is a compilation measurement, not a training-performance claim. The final small fence refinement was subsequently rebuilt and tested on Linux as well.

These results are local validation; GitHub CI for the newly pushed revision still needs to complete.

@MrNeRF MrNeRF left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Rebuilt it with SDK 10.1.0 (gfx1150, gfx942 and the 26-target compressed module); no AMD GPU here, so the GPU test groups didn't run.

Round 2 Now
26-target compressed build 829 s 533 s
26-target module 4.78 MB 4.56 MB
gfx1150 module 7.98 MB 7.56 MB
gfx1150 DeviceSelect code 275 KB 120 KB
gfx1150 kernels (> 8 B) 524 429
CPU CTest (each build) / Python 5/5 / 26/26 5/5 / 26/26

What's fixed:

  • One Bool rule: boolean_pointwise is gone.
  • Selection: it reads plain pointers per width, and the duplicate count kernel is gone.
  • No extra host reads: precounted masked_select, masked_scatter checks the count on the device, and index checks run inside the kernels.
  • Instantiations: 17 FP16 operations, the 24 CUDA conversion pairs, 4 scalar-left operations. where_scalar and the four scalar-left ops are right to keep, since the adapter calls them.
  • Shared functor header: it's now included instead of copied.
  • ABI 5: a real round-2 module is rejected with "expected 5, got 4" before any export is bound.
  • Build: the three translation units plus --offload-jobs=2 build in ~14 min at GitHub speed, well inside 60.

One thing before it can go in:

A staged readback can return values from an invalid index op as a success. Checked indexing now returns right after submission and leaves its error in the context flag. But lfs_hip_readback_wait (runtime.hip:295) only waits for the readback event and checks the copy error, and lfs_hip_readback_poll (:314) copies the staged bytes and sets ready=1 without looking at that flag. So invalid gather → readback → wait → poll hands back stale values from the skipped lanes as a good result, and the -2 only shows up on a later context sync. Before, gather rejected the indices before returning.

I reproduced it with a host-side probe that runs your actual host functions with simulated queues: wait=0, poll=0, ready=1, value=99, pending_error=1. Please carry the pending execution error through wait/poll (and the packed readback) before exposing the result, without attributing a later operation's error to an earlier snapshot. A regression for invalid indexing followed directly by a readback would cover it.

Smaller:

  • fill and affine (runtime_common.hpp:201) are compiled into all three objects, and arange_kernel (pointwise_runtime.hpp:48) into two. Defining each once saves ~1.3 KB per target.
  • The PR description still says 68 FP16 operations, a copied functor header and ABI 4. It should also describe when deferred index errors surface and that valid lanes are written even when others are invalid.

Copy link
Copy Markdown
Collaborator Author

Addressed the remaining review requests in 9abcf08.

Deferred errors belong to the readback snapshot

The reported failure was reproducible on the previous module: four new regression methods failed, with five failed assertions including the empty-readback/marker variants. They pass after this correction.

  • Ordinary readback and ordering markers copy the pending execution flag into slot-owned pinned storage on the producer stream, before recording their completion event. Wait/poll inspect that captured value rather than reading the context's current flag. An invalid operation submitted later therefore cannot poison an earlier valid snapshot.
  • Packed appends accumulate errors in a slot-owned four-byte device flag. This preserves faults from earlier appended ranges even when context synchronization consumes the global error and subsequent checked work resets that flag.
  • Wait/poll report the captured error before publishing bytes or ready=1. Rejected polling leaves the caller's destination unchanged and ready=0.
  • A drained null poll discards a failed snapshot while still returning its error, allowing reuse. Context synchronization consumes its pending error independently; it cannot validate an already failed snapshot.
  • Capture/storage remains owned through allocation, copy, synchronization and release failures. Failed capture is latched as a failed result, rather than allowing the payload to appear successful.

Ordinary readback/markers add no kernel, device allocation or host wait for error capture: only an asynchronous flag copy when checked work is pending. Packed accumulation uses its own small device flag, which is included in memory accounting.

Regressions cover invalid-index → readback → wait/poll, direct polling without a preceding wait, earlier-valid/later-invalid snapshots, context-error consumption, packed appends across reset, empty readbacks, ordering markers, masked-scatter cardinality errors, failed-result discard/reuse, and fault-injected ownership/recovery. Valid lanes may still have written before an index fault; this remains an asynchronous error contract, not transactional rollback.

Duplicate kernels and description

fill and affine are now defined once in runtime.hip; arange_kernel is defined once in pointwise.hip. Reductions and degenerate random generation call the shared fill through an internal host launcher returning hipError_t, preserving native failure codes across translation units.

The PR description now reflects 17 reachable FP16 operations, 24 conversion pairs, direct inclusion of shared functors, ABI 5 and the deferred-error/readback contract. Earlier measured build times, sizes and numerical comparisons remain explicitly marked as historical validation. The CI selector also now watches pointwise_lowering.hpp, which the generator reads.

ABI remains 5: this correction changes opaque internal state and error propagation, without changing exported signatures or descriptor layouts.

Verification

  • Windows, SDK 10.1.0, gfx1150: normal and instrumented modules rebuilt; 9/9 CTest groups passed. Focused operator/recovery groups were repeated after the final regression additions.
  • WSL/Linux, SDK 10.1.0: all 26 compressed targets compiled; 5/5 CPU CTest groups passed. No Linux GPU execution is claimed.
  • 71 CPU Python tests passed; clang-format 19 on the PR's 23 C/C++/HIP files, conversion checks on all three HIP translation units, whitespace, error-debt and actionlint checks passed.
  • The separate, unpublished Tensor adapter was aligned and its packed-ring publication checks were updated too. Its local checks passed: 39 common GPU contracts on HIP and Vulkan, plus one HIP-specific packed-snapshot regression; CPU 32 passed / 7 GPU-only skips, loader 9/9, native 9/9, Python 80/80, syntax checks 42 HIP-only/CUDA+HIP + 16 HIP-disabled, and an extended-module Linux build for gfx1150/gfx942 with CPU 5/5. These adapter changes are not included in this PR.

No new training-quality/performance, macOS runtime or CUDA GPU result is claimed. Remote CI for the new head still needs to complete.

@MrNeRF MrNeRF left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The readback fix works. I reran my host-side probe against the new functions: the invalid-index case that used to come back as wait=0, poll=0, ready=1, value=99 now gives wait=-2, poll=-2, ready=0 and leaves the destination untouched. The extended probe passes 44 of 44 cases (10 of 44 on the previous head), including 192 slot and stream reuse cycles. Those cover an earlier valid snapshot followed by a later invalid op, packed appends across a reset and after the context error was consumed, empty readbacks, ordering markers, and discard and reuse after a failed poll. It runs clean under ASan/UBSan, and no pinned, device or event resources leak. fill/affine and arange_kernel are each defined once now, and a launch error still reaches the caller with its native code. The CI selector picks up pointwise_lowering.hpp changes, and since the private API headers are unchanged apart from comments, staying on ABI 5 is right.

Rebuilt with SDK 10.1.0 from a clean export of this head: gfx1150 7.55 MB, the 26-target compressed module 4.55 MB, CPU CTest 5/5 on both, Python 26/26. There's still no AMD GPU here, so the GPU groups didn't run on my side.

Small thing, no change needed: the extra hipGetLastError() in the empty chain reduction is redundant but harmless.

Copy link
Copy Markdown
Collaborator Author

Removed the redundant hipGetLastError() from the empty chain-reduction path in 9878038. This is a one-line cleanup: lfs_hip_fill_internal() already returns the launch status, and the retained check(...) propagates its native error code.

Both Windows SDK 10.1.0/gfx1150 modules were rebuilt incrementally; 9/9 CTest groups passed, including empty sum/mean chain reductions and fault recovery. clang-format, whitespace and the error-debt gate passed. No ABI or descriptor change; no new Linux/macOS or training-performance result is claimed for this cleanup.

@MrNeRF
MrNeRF merged commit eecd2cc into MrNeRF:dev Oct 26, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev branch Applies to the dev branch only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants