Refactor: decompose run() into shared head + tail helpers#924
Conversation
Pull the byte-identical sub-sequences at the head and tail of both arches' run() into DeviceRunnerBase helpers. The heavily-divergent middle (register address setup, profiling flag bits, init_*, collector start/teardown, dep_gen, ffts, kernel launches) stays on the subclass. Net -17 lines. New base helpers: - validate_launch_aicpu_num(int) — range check vs PLATFORM_MAX_AICPU_THREADS - ensure_device_wall_buffer() — lazy-alloc the 8-byte device wall buffer - resolve_block_dim(int) — auto-resolve or validate, returns block_dim - prepare_runtime_for_launch(Runtime&, int block_dim, int launch_aicpu_num) — num_aicore range check + worker_count/aicpu_thread_num setup + handshake init loop + task function_bin_addr loop - sync_run_streams() — sync aicpu then aicore with timeout + good error context (stream id, device id, block_dim) on the timeout sentinel - read_device_wall_ns() — D2H readback into device_wall_ns_ with soft-warn fallback on rtMemcpy failure Each helper is a verbatim move of code that was identical between the two arches. No behavior change. Both arches built clean. a2a3 onboard smoke (dummy_task, alternating_matmul_add, prepared_callable suite, spmd_basic) 9/9 passed in 19s.
|
Warning Review limit reached
More reviews will be available in 54 minutes and 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the DeviceRunner::run method across the a2a3 and a5 architectures by extracting common, byte-identical setup and teardown sequences into helper methods in the DeviceRunnerBase class. These helper methods include validate_launch_aicpu_num, ensure_device_wall_buffer, resolve_block_dim, prepare_runtime_for_launch, sync_run_streams, and read_device_wall_ns. This change significantly reduces code duplication and improves maintainability. There are no review comments to address, and I have no additional feedback to provide.
2 participants
Summary
Pulls the byte-identical sub-sequences at the head and tail of both arches'
run()intoDeviceRunnerBasehelpers. The heavily-divergent middle (register address setup, profiling-flag bits,init_*, collector start/teardown, dep_gen, ffts, kernel launches) stays on each subclass.Net -17 lines across the four touched files, but each arch's
run()is now noticeably shorter (a2a3 -65 lines, a5 -54 lines on the head/tail alone).New base helpers
validate_launch_aicpu_num(int)PLATFORM_MAX_AICPU_THREADSensure_device_wall_buffer()resolve_block_dim(int)query_max_block_dimor pass throughvalidate_block_dim; returns concrete block_dimprepare_runtime_for_launch(Runtime&, int block_dim, int launch_aicpu_num)function_bin_addrloopsync_run_streams()read_device_wall_ns()device_wall_ns_with soft-warn fallbackEach helper is a verbatim move of code that was identical between the two arches.
What's left arch-specific in run()
init_aicore_register_addresses(...AicoreRegKind...)APIinit_aicore_register_addressesAPI, on-failure PMU disable-and-continue pathenable_dep_gen_), all collector start / teardown,init_l2_perf/init_tensor_dump/init_pmu/init_scope_statscalls (per-archKernelArgsfield assignments),init_runtime_args+ log/device_id setup,init_device_kernel_args, kernel launchesThese are real per-arch differences and would need virtuals or templating to unify — out of scope here.
Verification
Test plan