Skip to content

[Code Health] Separate a run's input staging, result copy-back and binding release (Track A prerequisite) #2291

Description

@ChaoWao

Category

Technical Debt (cleanup, refactor)

Component

Host Runtime

Description

validate_runtime_impl(Runtime *, const HostApi *, int execution_rc) is one
entry doing three things with three different lifetimes:

  1. reading the run's device-side status (on the failure path);
  2. copying this run's outputs back through its TensorLease bindings;
  3. releasing those bindings.

And the inputs go the other way: copy_in_device_args
(tensormap_and_ringbuffer/host/runtime_maker.cpp:514, called from the bind at
:844) copies the caller's input bytes into the device argument slice while
resolving the arguments, so a run's input staging happens during preparation
rather than at its own submission.

Three consequences on main today:

  • A control fact is communicated by mutating data. The two never-launched
    paths tell the status read to skip its device access by writing
    set_gm_sm_ptr(nullptr) into the caller's Runtime
    (c_api_shared.cpp:796, :1155). So a run that fails before launch mutates
    the image it was about to publish, and the read side cannot be const.
    "Never launched" is a property of the submission, not of the runtime image.
  • Release is welded to copy-back, so a run cannot have its results read and
    its device bindings retired at different times. [Code Health] Use per-run dual-stream completion fences (Track A P2) #2267's partial-submission
    cases need exactly that: kernel submitted, event missing → retain
    device-referenced resources until an independent quiescence proof
    . With one
    entry there is no seam to retain them at.
  • Input staging belongs to preparation. Under [Feature] Let consecutive runs queue on the same stream instead of round-tripping through a host sync #1853's queueing a successor
    is prepared while its predecessor executes, and "each invocation prepares its
    own inputs and submits its execution context once" is the agreed shape. The
    bind is the wrong place for the H2D of a run's input bytes; recording which
    buffer each argument uses is the bind's job, and staging the bytes is the
    submission's.

This is the in-scope remainder of the withdrawn reusable-PreparedCall
milestone (#2270), carved down to what serves Pipeline A. Nothing here retains
a prepared-call store, reuse controls, a cross-invocation preparation cache or
any interface for resubmitting a prepared context — those are withdrawn
requirements.

Location

Source reference: 44dccbef8 (main after #2283). Recheck current main before
implementation.

  • src/{a2a3,a5}/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp:
    copy_in_device_args (:514), the bind (:844), validate_runtime_impl
    (:926), read_runtime_status, read_published_run_status.
  • src/{a2a3,a5}/runtime/host_build_graph/host/runtime_maker.cpp:
    validate_runtime_impl (a2a3 :1435, a5 :2090), and the
    set_gm_sm_ptr(nullptr) at a2a3 :1334 / a5 :1989.
  • src/common/platform/onboard/host/c_api_shared.cpp:
    cleanup_failed_prepare (:790) and simpler_finalize_run (:1155).
  • src/common/platform/sim/host/c_api_shared.cpp: the same two call sites.
  • src/common/worker/runtime_c_api.h: the runtime-impl symbol list.
  • src/common/utils/tensor_lease.h: TensorLease, TensorReleaseKind.
  • Existing tests: tests/ut/cpp/common/test_trb_runtime_temp_buffer.cpp
    (compiles the real runtime_maker.cpp against a fake HostApiOps, host-only,
    per arch).

Proposed Fix

Three extern "C" entries in place of one, provided by both runtimes:

int copy_in_run_inputs_impl(const Runtime *, const HostApi *);
int copy_back_run_outputs_impl(const Runtime *, const HostApi *, int execution_rc, int launched);
int release_run_bindings_impl(Runtime *, const HostApi *);
  • copy_in_device_args stops copying bytes and records needs_copy_in on the
    lease alongside the existing needs_copy_back. copy_in_run_inputs_impl
    performs the per-run H2D and returns the first failing rc.
    host_build_graph's is a documented no-op: its host orchestrator reads the
    input tensors while it builds the graph, so its inputs are staged inside its
    own bind.
  • launched becomes an argument. No path writes set_gm_sm_ptr(nullptr), and
    the read side (read_runtime_status, arm_collectors_for_run,
    init_args_dump, the get_workers() accessors) takes const Runtime *.
  • Release stays adjacent to copy-back at today's call sites. This issue
    changes no normal-path behavior
    — its deliverable is the seam plus the
    removal of the data mutation, not a reordering. Deferred release under
    partial submission belongs to [Code Health] Use per-run dual-stream completion fences (Track A P2) #2267.

Internal boundary, both sides in this repo, so no version field.

Acceptance criteria

Each names an observation, and what the wrong value would mean:

  1. A failed prepare leaves the caller's runtime image alone. After a prepare
    that fails before launch, the caller's Runtime compares equal to what it
    held before — in particular gm_sm_ptr() is unchanged. A changed value
    means the control fact is still travelling as a data mutation.
  2. No device status read without a launch. With launched == 0, the fake
    HostApi's device-read count against the shared-memory region is 0. A
    nonzero count means the skip is still inferred from a nulled pointer rather
    than from the argument.
  3. Copy-back does not retire bindings. After copy_back_run_outputs_impl,
    the fake ops' free count is 0 and the lease vector is unchanged; after
    release_run_bindings_impl it equals the number of Free-kind leases. A
    nonzero count at the first check means release is still welded to copy-back.
  4. Input bytes are staged per execution, not at bind. The bind's H2D count
    against lease destinations is 0, and copy_in_run_inputs_impl issues
    exactly one H2D per needs_copy_in lease. A nonzero bind count means input
    staging is still preparation-time work. packed_temp_bytes and the
    descriptor/scalar content the bind writes are unchanged by the move.
  5. Direction semantics are preserved. An IN lease is copied in and not
    back; an OUT lease is copied back and not in; an INOUT lease is both.
    Asserted per direction rather than by a total count, so a flag inverted on
    one direction cannot pass.
  6. A failed input staging keeps the run's bindings. If
    copy_in_run_inputs_impl fails, the leases are still present and releasable,
    and the failure is the rc the caller sees.
  7. Production regressions. Full onboard a2a3 and a5 scene suites for both
    runtimes, both sim corpora, cpput, pyut, and the per-channel DFX lanes
    (--enable-chip-swimlane, --dump-args, --enable-pmu, --enable-dep-gen,
    --enable-scope-stats) — the --dump-args channel specifically, because it
    reads the argument slice this change moves the bytes into.

Scope boundaries

Not in this issue: #2267's completion fences and deferred release under partial
submission, #1853's admission widening, #2257's KernelArgs payload change, any
retained or reusable prepared result, and any change to native tensor access
policy.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions