[Fixup] Reflow comments to 120-col budget; drop unit-test reference in program.h

Author: duburcqa

quadrants/program/program.cpp

@@ -362,10 +362,9 @@ Program::AdStackOverflowDiagnosis Program::diagnose_adstack_overflow(uint32_t ta
           }
           int64_t v = -1;
           if (host_resolvable && !expr.nodes.empty()) {
-            // `evaluate_adstack_size_expr` reads the live SNode state via `SNodeRwAccessorsBank` and
-            // const-folds the rest in plain C++. `ctx == nullptr` is safe because every leaf we kept
-            // is host-resolvable; ETS / ETR are the only kinds that touch ctx and we filtered them
-            // out.
+            // `evaluate_adstack_size_expr` reads the live SNode state via `SNodeRwAccessorsBank` and const-folds the
+            // rest in plain C++. `ctx == nullptr` is safe because every leaf we kept is host-resolvable; ETS / ETR
+            // are the only kinds that touch ctx and we filtered them out.
             SizeExprLaunchScope scope;
             v = evaluate_adstack_size_expr(expr, const_cast<Program *>(this), nullptr);
           }

quadrants/program/program.h

@@ -235,12 +235,11 @@ class QD_DLL_EXPORT Program {
   // Identity-key for idempotent re-registration. The diagnose path NEVER dereferences this pointer;
   // it stores all size-expression data inline (`size_exprs`) so the entry is self-contained and
   // immune to lifetime issues from the underlying `AdStackSizingInfo` (LLVM) /
-  // `AdStackSizingAttribs` (SPIR-V) struct moves. Two structs because LLVM and SPIR-V backends store
-  // launch-time strides differently (byte-indexed vs element-indexed) and SPIR-V doesn't carry the
-  // dynamic-range-for fields LLVM needs - unifying them would be a backend-pipeline refactor far
-  // outside this PR's scope. Inlining `size_exprs` into the registry is the cheapest way to make
-  // the registry uniform across both backends without touching the launchers' on-the-hot-path
-  // structs.
+  // `AdStackSizingAttribs` (SPIR-V) struct moves. Two structs because LLVM and SPIR-V backends store launch-time
+  // strides differently (byte-indexed vs element-indexed) and SPIR-V doesn't carry the dynamic-range-for fields LLVM
+  // needs - unifying them would be a backend-pipeline refactor far outside this PR's scope. Inlining `size_exprs`
+  // into the registry is the cheapest way to make the registry uniform across both backends without touching the
+  // launchers' on-the-hot-path structs.
   struct AdStackSizingInfoEntry {
     const void *identity_key{nullptr};
     std::string kernel_name;
@@ -253,10 +252,9 @@ class QD_DLL_EXPORT Program {
                                         int task_id_in_kernel,
                                         std::vector<int> allocated_max_sizes,
                                         std::vector<SerializedSizeExpr> size_exprs);
-  // Refresh just the `size_exprs` snapshot in an existing registry entry. Used by the LLVM launcher
-  // on the first launch of a task whose codegen-time registration could not capture size_exprs (the
-  // codegen-time `current_task->ad_stack` had not yet been finalized). No-op for `id == 0` and ids
-  // outside the registry range.
+  // Refresh just the `size_exprs` snapshot in an existing registry entry. Used by the LLVM launcher on the first
+  // launch of a task whose codegen-time registration could not capture size_exprs (the codegen-time
+  // `current_task->ad_stack` had not yet been finalized). No-op for `id == 0` and ids outside the registry range.
   void update_adstack_sizing_info_size_exprs(uint32_t id, std::vector<SerializedSizeExpr> size_exprs);
   // Returns a *copy* of the registry entry (not a pointer into the underlying vector) so the caller can
   // safely hold the data across operations that might trigger another `register_adstack_sizing_info` and
@@ -278,9 +276,8 @@ class QD_DLL_EXPORT Program {
     bool confirmed_invalid_cache{false};
   };
   AdStackOverflowDiagnosis diagnose_adstack_overflow(uint32_t task_id) const;
-  // Convenience wrapper retained for the C++ unit test surface that exercises just the message
-  // formatting; production code uses `diagnose_adstack_overflow` to also act on the confirmed-cause
-  // signal.
+  // Convenience wrapper that returns just the message string; production code uses `diagnose_adstack_overflow` to
+  // also act on the confirmed-cause signal.
   std::string diagnose_adstack_overflow_message(uint32_t task_id) const;
 
   /**

quadrants/runtime/cpu/kernel_launcher.h

@@ -21,10 +21,9 @@ class KernelLauncher : public LLVM::KernelLauncher {
     // serial), so no launch-time gtmp resolution is needed on this backend.
     std::vector<AdStackSizingInfo> ad_stacks;
     std::vector<std::size_t> num_threads_per_task;
-    // Per-task snode-write set / arg-write set, copied off `OffloadedTask::snode_writes` /
-    // `arr_writes` at register time. Used by `launch_llvm_kernel` to bump
-    // `Program::snode_write_gen_` / `ndarray_data_gen_` before each launch so the per-task adstack
-    // metadata cache invalidates when this kernel mutates a SNode / ndarray a downstream
+    // Per-task snode-write set / arg-write set, copied off `OffloadedTask::snode_writes` / `arr_writes` at register
+    // time. Used by `launch_llvm_kernel` to bump `Program::snode_write_gen_` / `ndarray_data_gen_` before each launch
+    // so the per-task adstack metadata cache invalidates when this kernel mutates a SNode / ndarray a downstream
     // `size_expr` reads.
     std::vector<std::vector<int>> snode_writes_per_task;
     std::vector<std::vector<int>> arr_writes_per_task;

quadrants/runtime/gfx/runtime.cpp

@@ -1037,9 +1037,8 @@ void GfxRuntime::synchronize() {
       if (prog != nullptr) {
         auto diag = prog->diagnose_adstack_overflow(task_id_val);
         diagnostic = std::move(diag.message);
-        // See `LlvmRuntimeExecutor::check_adstack_overflow` for the rationale; only invalidate when
-        // the sizer rerun confirmed a stale cache (DLPack-bypass) so a Quadrants pre-pass bug is
-        // not silently masked.
+        // See `LlvmRuntimeExecutor::check_adstack_overflow` for the rationale; only invalidate when the sizer rerun
+        // confirmed a stale cache (DLPack-bypass) so a Quadrants pre-pass bug is not silently masked.
         if (diag.confirmed_invalid_cache) {
           prog->adstack_cache().invalidate_all();
         }

quadrants/runtime/llvm/llvm_adstack_lazy_claim.cpp

@@ -642,11 +642,10 @@ void LlvmRuntimeExecutor::check_adstack_overflow() {
   if (flag == 0) {
     return;
   }
-  // Drain the companion task-id slot in the same poll. Both slots cleared so the next overflow records
-  // a fresh identity. `task_id == 0` means the kernel that overflowed pre-dates the registry wiring or
-  // its `ad_stack.registry_id` was unset for any reason (e.g. a deserialised offline-cache task that has
-  // not yet been re-registered); the diagnose helper falls through to the generic dual-cause message in
-  // that case.
+  // Drain the companion task-id slot in the same poll. Both slots cleared so the next overflow records a fresh
+  // identity. `task_id == 0` means the kernel that overflowed pre-dates the registry wiring or its
+  // `ad_stack.registry_id` was unset for any reason (e.g. a deserialised offline-cache task that has not yet been
+  // re-registered); the diagnose helper falls through to the generic dual-cause message in that case.
   uint32_t task_id = 0;
   if (adstack_overflow_task_id_host_ptr_ != nullptr) {
     int64_t recorded = __atomic_exchange_n(adstack_overflow_task_id_host_ptr_, (int64_t)0, __ATOMIC_RELAXED);
@@ -667,8 +666,8 @@ void LlvmRuntimeExecutor::check_adstack_overflow() {
     }
   } else {
     diagnostic =
-        "Adstack overflow: a reverse-mode autodiff kernel pushed more elements than the adstack "
-        "capacity allows.";
+        "Adstack overflow: a reverse-mode autodiff kernel pushed more elements than the adstack capacity "
+        "allows.";
   }
   throw QuadrantsAssertionError(
       "Adstack overflow: a reverse-mode autodiff kernel pushed more elements "

tests/cpp/program/test_diagnose_adstack_overflow.cpp

@@ -10,8 +10,8 @@ namespace quadrants::lang {
 TEST(DiagnoseAdstackOverflow, RegistryAndLookup) {
   Program prog(host_arch());
 
-  // Two distinct sizing infos. The pointer is just an identity key for `Program::register_*` -
-  // the test does not deref.
+  // Two distinct sizing infos. The pointer is just an identity key for `Program::register_*` - the test does not
+  // deref.
   int dummy_a = 0;
   int dummy_b = 0;
   uint32_t id_a = prog.register_adstack_sizing_info(static_cast<const void *>(&dummy_a), "kernel_a", /*task=*/0,

tests/python/test_adstack.py

@@ -1226,10 +1226,9 @@ def test_adstack_overflow_diagnostic_and_auto_recovery():
     #   3. The next reverse launch reads cached `max_size = 2`, pushes 64, overflows. The host poll
     #      raises with the enriched diagnostic naming kernel + offload-task index. The raise site
     #      ALSO bulk-invalidates the adstack-sizer caches on its way out.
-    #   4. The user catches the exception. They do NOT need to manually adjust `ad_stack_size`. On
-    #      the NEXT reverse launch, the sizer reruns from scratch (cache invalidated), reads the
-    #      mutated `n[0] = 64`, sizes capacity to 64, and the kernel runs to completion with the
-    #      correct gradient.
+    #   4. The user catches the exception. They do NOT need to manually adjust `ad_stack_size`. On the NEXT reverse
+    #      launch, the sizer reruns from scratch (cache invalidated), reads the mutated `n[0] = 64`, sizes capacity
+    #      to 64, and the kernel runs to completion with the correct gradient.
     #
     # This auto-recovery contract is what lets the user's training-loop code recover from a
     # transient cache-staleness window without per-iteration retries: the offending data has
@@ -1286,8 +1285,8 @@ def compute(n_arr: qd.types.ndarray(dtype=qd.i32, ndim=1)):
     # LLVM-CPU the host-eval branch always re-evaluates the size expression per launch via
     # `try_size_expr_cache_hit`, which observes the live ndarray read and self-invalidates on
     # mismatch - that path never raises here, so the second compute.grad call returns cleanly. The
-    # `pytest.raises if shortcut else nullcontext` pattern handles both paths uniformly without
-    # arch-narrowing the test.
+    # `pytest.raises if shortcut else nullcontext` pattern handles both paths uniformly without arch-narrowing the
+    # test.
     y[None] = 0.0
     compute(n)
     y.grad[None] = 1.0