Skip to content

fix(rocm): key custom-kernel JIT modules by their generated source #2149

Closed

Description

@inureyes

Part of #1801

Problem / Background

Found in #2100's security review. The vendored ROCm fast::hip_kernel caches compiled modules under a key that does not cover everything the generated source depends on, so a second call with different input dtypes can silently reuse a module compiled for the first call and read its buffers through the wrong pointer type. Nothing throws; the output is just wrong.

Current Behavior

In src/lib/mlx-cpp/patches-rocm/mlx/backend/rocm/custom_kernel.cpp:

  • :265-266 names the kernel "custom_kernel_" + name + template_arguments_hash(template_args); template_arguments_hash (:108) hashes only the template args.
  • build_kernel (:129-202) generates a source that also depends on: each input's dtype (dtype_to_hip_type(arr.dtype()), :150), each output's dtype (:171), and whether each input has ndim() > 0, which decides whether its _shape / _strides / _ndim parameters are declared at all (:153-164). The ndim value itself does not matter: since LOCAL_FIXES.md item 30, <input>_shape is a fixed-width KernelShape passed by value.
  • CustomKernel::eval_gpu (:346-354) calls rocm::get_jit_module(s.device, name_, builder, false), and get_jit_module (jit_module.cpp:502-517) memoises by device.index + ":" + name and runs the builder only on a miss.

Upstream CUDA at the MLX pin 81ba1c6a already fixed this: mlx/backend/cuda/custom_kernel.cpp keys the module by fmt::format("{}_{:x}", name_, std::hash<std::string>{}(source_)) (around line 313), and Metal appends every input dtype to the name. The ROCm fork keeps the old name-only key.

mlxcel's own launchers are mostly shielded by scripts/ci/check_kernel_dtype_keys.py (make verify-kernel-dtype-keys), which requires at least one input dtype in each launch's template_args. It cannot cover a launch whose inputs have different dtypes that vary independently, an output dtype that differs at one input dtype, or a scalar versus non-scalar input, and its docstring (:15-19) still describes CUDA as name-only, which is stale at the pin.

Proposed Solution

Mirror upstream CUDA in the overlay: in CustomKernel::eval_gpu, pass module_name = name_ + "_" + hex(std::hash<std::string>{}(source_)) to rocm::get_jit_module while keeping kernel_name (the symbol looked up in the module) unchanged. Because the source is the complete input to compilation, this covers dtypes, the scalar case and any future source-affecting input at once. Add a LOCAL_FIXES.md entry citing the CUDA line it copies. Leave get_jit_module's device-index prefix as is.

Rejected: appending input and output dtypes to the name as Metal does. It fixes dtypes but not the scalar-input case, and diverges from the backend the overlay tracks.

Also correct the CUDA paragraph of check_kernel_dtype_keys.py's docstring (the check itself stays as defense in depth for a future backend or a re-vendored fork that regresses the key).

Implementation Notes

  • Reuse: the rocm_fault_probe_array bridge fixture (src/lib/mlxcel-core/cpp/mlx_cxx_bridge.cpp:5163, wrapper mlxcel_core::rocm_faults) is the pattern for a ROCm-only test kernel reachable from Rust.
  • Edge cases: the hash is computed per eval_gpu; source_ is built once per call in hip_kernel's closure, so the extra cost is one string hash per launch, the same as CUDA. Two sources that collide in std::hash would still share a module; CUDA accepts that and so does this fix.
  • Constraints: no change for Metal or CUDA builds; no change to kernel symbol names, so rocprofv3 traces keep their custom_kernel_* names.

Acceptance Criteria

  • A new ROCm bridge fixture rocm_jit_key_probe(input) -> Result<UniquePtr<MlxArray>> launches one hip_kernel with a fixed name and no template args that writes out[i] = (float)inp[i] into an f32 output.
  • tests/rocm_custom_kernel_jit_key.rs calls it with an f32 array, then an f16 array, then a bf16 array of known values, and asserts each output equals the input. It fails on current main (second and third calls reuse the f32 module) and passes with the fix.
  • The same test covers a 0-d input after a 1-d input with the probe reading inp_shape, run in a child process as tests/rocm_gpu_faults.rs does (on current main the argument lists disagree and can fault the queue), and passes with the fix.
  • make verify-rocm passes with no other test change; LOCAL_FIXES.md and make verify-rocm-overlay agree.

Verification

cargo test --release --features rocm --test rocm_custom_kernel_jit_key -- --test-threads=1
make verify-rocm verify-kernel-dtype-keys

Activity

  1. added
    type:bugBug fixes, error corrections, or issue resolutions
    area:coremlxcel-core: MLX FFI, primitives, KV cache, layers
    platform:linuxLinux (CUDA / packaging) specific
    on Oct 6, 2026
  2. added a commit that references this issue on Oct 14, 2026
    9c0ae2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:coremlxcel-core: MLX FFI, primitives, KV cache, layersplatform:linuxLinux (CUDA / packaging) specificpriority:mediumMedium prioritystatus:doneCompletedtype:bugBug fixes, error corrections, or issue resolutions

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions