[Fixup] Restore upstream comment formatting and fflush(stdout) in LlvmRuntimeExecutor::synchronize

Author: duburcqa

quadrants/program/program.h

@@ -202,8 +202,10 @@ class QD_DLL_EXPORT Program {
     return snode_rw_accessors_bank_;
   }
 
-  // Resolve an `SNode *` from its global `SNode::id`. Lazy `snode_id_cache_` lookup; the cache is wiped by
-  // `add_snode_tree` and `destroy_snode_tree`.
+  // Look up an `SNode` in this `Program`'s snode trees by its global `SNode::id`. Used by the host-side adstack
+  // size-expression evaluator to rehydrate an `SNode *` from a `snode_id` that survived the offline cache. Linear
+  // over all snode trees; called at most once per adstack leaf per kernel launch so the cost is negligible in
+  // practice.
   SNode *get_snode_by_id(int snode_id);
 
   // One input read observed during a `evaluate_adstack_size_expr` walk. The cache entry records these so

quadrants/program/program_impl.h

@@ -143,8 +143,9 @@ class ProgramImpl {
   virtual void finalize() {
   }
 
-  // Invoked by `Program::finalize()` before any teardown sync, so the backend can flip state that
-  // silences user-visible asserts during destructor unwinding.
+  // Hook invoked by `Program::finalize()` before any teardown sync. Lets backends flip state (e.g. the LLVM
+  // `finalizing_` flag used to suppress adstack-overflow polling) so the two `Program::synchronize()` calls that
+  // precede `finalize()` do not throw into the Program destructor path.
   virtual void pre_finalize() {
   }
 

quadrants/rhi/metal/metal_device.h

@@ -175,15 +175,18 @@ struct MetalShaderBindingMapping {
   // Map GLSL binding to MSL index (buffer/texture index, sampler index)
   std::unordered_map<int, std::pair<int, int>> vertex;
   std::unordered_map<int, std::pair<int, int>> fragment;
-  // The highest used buffer index used in the MSL vertex function Vertex attributes use up vertex buffer indices in
-  // Metal, but SPIRV-cross generated MSL doesn't know how many buffer indices the vertex attributes will take. So bind
-  // vertex input after the other buffers.
+  // The highest used buffer index used in the MSL vertex function
+  // Vertex attributes use up vertex buffer indices in Metal, but
+  // SPIRV-cross generated MSL doesn't know how many buffer indices
+  // the vertex attributes will take. So bind vertex input after the other
+  // buffers.
   int max_vert_buffer_index{-1};
 };
 
 class MetalPipeline final : public Pipeline, public rhi_impl::NonAssignable {
  public:
-  // `mtl_library`, `mtl_function`, `mtl_compute_pipeline_state` should be already retained.
+  // `mtl_library`, `mtl_function`, `mtl_compute_pipeline_state` should be
+  // already retained.
   explicit MetalPipeline(const MetalDevice &device,
                          MTLLibrary_id mtl_library,
                          MTLFunction_id mtl_function,
@@ -379,7 +382,8 @@ class MetalCommandList final : public CommandList {
   void set_line_width(float width) override;
 
   MTLCommandBuffer_id finalize();
-  // If noclear is false, ignore whatever is set in details This may be used to "resume" the current renderpass
+  // If noclear is false, ignore whatever is set in details
+  // This may be used to "resume" the current renderpass
   MTLRenderPassDescriptor *create_render_pass_desc(bool depth_write, bool noclear = false);
 
   bool is_renderpass_active() const;
@@ -405,12 +409,12 @@ class MetalCommandList final : public CommandList {
   std::vector<MTLTexture_id> render_targets_;
   MTLTexture_id depth_target_;
 
-  // Buffers accessed via physical storage buffer pointers in the next dispatch. Needed for Metal hazard tracking since
-  // these are not explicitly bound.
+  // Buffers accessed via physical storage buffer pointers in the next dispatch.
+  // Needed for Metal hazard tracking since these are not explicitly bound.
   std::vector<DeviceAllocation> tracked_physical_buffers_;
 
-  // For renderpass resuming, track whether a renderpass has been started Used to override LoadAction, to prevent
-  // uninteded clearing when resuming
+  // For renderpass resuming, track whether a renderpass has been started
+  // Used to override LoadAction, to prevent uninteded clearing when resuming
   bool is_renderpass_active_{false};
 
   // Persistent compute command encoder reused across consecutive `dispatch()` calls so the GPU sees one

quadrants/runtime/llvm/llvm_runtime_executor.cpp

@@ -207,6 +207,7 @@ void LlvmRuntimeExecutor::synchronize() {
     QD_ERROR("No AMDGPU support");
 #endif
   }
+  fflush(stdout);
 }
 
 uint64 LlvmRuntimeExecutor::fetch_result_uint64(int i, uint64 *result_buffer) {

quadrants/runtime/program_impls/llvm/llvm_program.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include <cstddef>
-#include <cstdio>
 #include <memory>
 
 #include "quadrants/runtime/llvm/llvm_offline_cache.h"
@@ -100,8 +99,11 @@ class LlvmProgramImpl : public ProgramImpl {
     return runtime_exec_->fetch_result<T>(i, result_buffer);
   }
 
-  // Mark the impl as shutting down so `synchronize_and_assert()` skips the overflow poll, preventing a
-  // `qd.sync()` racing with destruction from throwing inside `~Program()` unwinding.
+  // Skip the adstack-overflow poll from this point on: `Program::finalize()` invokes `pre_finalize()` before the
+  // two teardown `synchronize()` calls, and we do not want `check_adstack_overflow()` to throw into a
+  // `~Program()` unwinding path - that would terminate the process with a bare `QuadrantsAssertionError` instead
+  // of letting the user handle it at their own `qd.sync()` site. The flag only affects the internal poll; the
+  // user can still call `qd.sync()` explicitly before finalize to observe the raise.
   void pre_finalize() override {
     finalizing_ = true;
   }
@@ -165,8 +167,6 @@ class LlvmProgramImpl : public ProgramImpl {
     if (!finalizing_) {
       runtime_exec_->check_adstack_overflow();
     }
-    // Flush JIT-generated `qd.print()` output at every user-visible sync.
-    fflush(stdout);
   }
 
   LLVMRuntime *get_llvm_runtime() {
@@ -267,8 +267,11 @@ class LlvmProgramImpl : public ProgramImpl {
   std::size_t num_snode_trees_processed_{0};
   std::unique_ptr<LlvmRuntimeExecutor> runtime_exec_;
   std::unique_ptr<LlvmOfflineCache> cache_data_;
-  // Set by `pre_finalize()` / `finalize()` so `synchronize_and_assert()` skips the overflow poll while
-  // teardown is in progress.
+  // Flipped on by `pre_finalize()` (with a defensive re-assignment in `finalize()`) so the `synchronize()`
+  // override stops polling the adstack-overflow flag during teardown. `Program::finalize()` invokes
+  // `pre_finalize()` before its two teardown syncs, so the flag is already true when those syncs run; moving
+  // the assignment back into `finalize()` alone would silently re-introduce the `std::terminate()` teardown bug
+  // this field was introduced to fix.
   bool finalizing_{false};
 };