Skip to content

Commit eb9fda7

Browse files
committed
[Fixup] Restore upstream comment formatting and fflush(stdout) in LlvmRuntimeExecutor::synchronize
1 parent 42e8ae8 commit eb9fda7

4 files changed

Lines changed: 18 additions & 11 deletions

File tree

quadrants/program/program.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,10 @@ class QD_DLL_EXPORT Program {
201201
return snode_rw_accessors_bank_;
202202
}
203203

204-
// Resolve an `SNode *` from its global `SNode::id`. Lazy `snode_id_cache_` lookup; the cache is wiped by
205-
// `add_snode_tree` and `destroy_snode_tree`.
204+
// Look up an `SNode` in this `Program`'s snode trees by its global `SNode::id`. Used by the host-side adstack
205+
// size-expression evaluator to rehydrate an `SNode *` from a `snode_id` that survived the offline cache. Linear
206+
// over all snode trees; called at most once per adstack leaf per kernel launch so the cost is negligible in
207+
// practice.
206208
SNode *get_snode_by_id(int snode_id);
207209

208210
// Adstack-specific caching: per-task adstack-sizer metadata caches (SPIR-V + LLVM-GPU), encoded SPIR-V bytecode

quadrants/program/program_impl.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,9 @@ class ProgramImpl {
143143
virtual void finalize() {
144144
}
145145

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

quadrants/runtime/llvm/llvm_runtime_executor.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ void LlvmRuntimeExecutor::synchronize() {
212212
QD_ERROR("No AMDGPU support");
213213
#endif
214214
}
215+
fflush(stdout);
215216
}
216217

217218
uint64 LlvmRuntimeExecutor::fetch_result_uint64(int i, uint64 *result_buffer) {

quadrants/runtime/program_impls/llvm/llvm_program.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#pragma once
22

33
#include <cstddef>
4-
#include <cstdio>
54
#include <memory>
65

76
#include "quadrants/runtime/llvm/llvm_offline_cache.h"
@@ -100,8 +99,11 @@ class LlvmProgramImpl : public ProgramImpl {
10099
return runtime_exec_->fetch_result<T>(i, result_buffer);
101100
}
102101

103-
// Mark the impl as shutting down so `synchronize_and_assert()` skips the overflow poll, preventing a
104-
// `qd.sync()` racing with destruction from throwing inside `~Program()` unwinding.
102+
// Skip the adstack-overflow poll from this point on: `Program::finalize()` invokes `pre_finalize()` before the
103+
// two teardown `synchronize()` calls, and we do not want `check_adstack_overflow()` to throw into a
104+
// `~Program()` unwinding path - that would terminate the process with a bare `QuadrantsAssertionError` instead
105+
// of letting the user handle it at their own `qd.sync()` site. The flag only affects the internal poll; the
106+
// user can still call `qd.sync()` explicitly before finalize to observe the raise.
105107
void pre_finalize() override {
106108
finalizing_ = true;
107109
}
@@ -165,8 +167,6 @@ class LlvmProgramImpl : public ProgramImpl {
165167
if (!finalizing_) {
166168
runtime_exec_->check_adstack_overflow();
167169
}
168-
// Flush JIT-generated `qd.print()` output at every user-visible sync.
169-
fflush(stdout);
170170
}
171171

172172
LLVMRuntime *get_llvm_runtime() {
@@ -267,8 +267,11 @@ class LlvmProgramImpl : public ProgramImpl {
267267
std::size_t num_snode_trees_processed_{0};
268268
std::unique_ptr<LlvmRuntimeExecutor> runtime_exec_;
269269
std::unique_ptr<LlvmOfflineCache> cache_data_;
270-
// Set by `pre_finalize()` / `finalize()` so `synchronize_and_assert()` skips the overflow poll while
271-
// teardown is in progress.
270+
// Flipped on by `pre_finalize()` (with a defensive re-assignment in `finalize()`) so the `synchronize()`
271+
// override stops polling the adstack-overflow flag during teardown. `Program::finalize()` invokes
272+
// `pre_finalize()` before its two teardown syncs, so the flag is already true when those syncs run; moving
273+
// the assignment back into `finalize()` alone would silently re-introduce the `std::terminate()` teardown bug
274+
// this field was introduced to fix.
272275
bool finalizing_{false};
273276
};
274277

0 commit comments

Comments
 (0)