From a8b70295e245ae8e0d0ae79cacc2fd40ee6faa9d Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Thu, 6 Aug 2026 15:44:55 -0700 Subject: [PATCH 01/13] Reject GPU register allocations shared between threads Register and stack memory is private to a GPU thread, so an allocation of one outside the loops over threads gives every thread its own copy of the whole thing rather than one copy shared by the block. A thread that reads a part of it another thread computed reads its own uninitialized copy of that part instead. Halide accepted such schedules and quietly computed the wrong answer. Check that each thread keeps to its own part of the allocation, in the manner of can_parallelize_rvar: describe the same access made by some other thread, and require that the two can never meet. Only the variables bound at or inside the loops over threads are renamed to make the other thread, because the rest, such as the base of the block's tile, are shared by the whole block. The check has to sit between storage folding and storage flattening, which is later in lowering than validation usually goes. See the comment in CheckGPUCrossTalk.h. Also lifts RenameFreeVars and the substitution of boolean lets out of ParallelRVar.cpp, which is where this technique already lives. Co-Authored-By: Claude Opus 5 --- Makefile | 2 + src/CMakeLists.txt | 2 + src/CheckGPUCrossTalk.cpp | 269 ++++++++++++++++++++++++++ src/CheckGPUCrossTalk.h | 35 ++++ src/Lower.cpp | 7 + src/ParallelRVar.cpp | 41 +--- src/Substitute.cpp | 38 ++++ src/Substitute.h | 22 +++ test/error/CMakeLists.txt | 1 + test/error/gpu_register_crosstalk.cpp | 25 +++ 10 files changed, 402 insertions(+), 40 deletions(-) create mode 100644 src/CheckGPUCrossTalk.cpp create mode 100644 src/CheckGPUCrossTalk.h create mode 100644 test/error/gpu_register_crosstalk.cpp diff --git a/Makefile b/Makefile index 44b98aa9b998..bbc8a49b71fc 100644 --- a/Makefile +++ b/Makefile @@ -464,6 +464,7 @@ SOURCE_FILES = \ Buffer.cpp \ Callable.cpp \ CanonicalizeGPUVars.cpp \ +CheckGPUCrossTalk.cpp \ ClampUnsafeAccesses.cpp \ Closure.cpp \ CodeGen_ARM.cpp \ @@ -667,6 +668,7 @@ HEADER_FILES = \ Buffer.h \ Callable.h \ CanonicalizeGPUVars.h \ +CheckGPUCrossTalk.h \ ClampUnsafeAccesses.h \ Closure.h \ CodeGen_C.h \ diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7727588fe764..aa86aa833a49 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -74,6 +74,7 @@ target_sources( Buffer.h Callable.h CanonicalizeGPUVars.h + CheckGPUCrossTalk.h ClampUnsafeAccesses.h Closure.h CodeGen_C.h @@ -257,6 +258,7 @@ target_sources( Buffer.cpp Callable.cpp CanonicalizeGPUVars.cpp + CheckGPUCrossTalk.cpp ClampUnsafeAccesses.cpp Closure.cpp CodeGen_ARM.cpp diff --git a/src/CheckGPUCrossTalk.cpp b/src/CheckGPUCrossTalk.cpp new file mode 100644 index 000000000000..6aee4fa1f0f0 --- /dev/null +++ b/src/CheckGPUCrossTalk.cpp @@ -0,0 +1,269 @@ +#include "CheckGPUCrossTalk.h" + +#include "CSE.h" +#include "CanonicalizeGPUVars.h" +#include "IR.h" +#include "IROperator.h" +#include "IRPrinter.h" +#include "IRMutator.h" +#include "IRVisitor.h" +#include "Simplify.h" +#include "Substitute.h" + +#include +#include + +namespace Halide { +namespace Internal { + +using std::string; +using std::vector; + +namespace { + +// One loop over GPU threads. The producer and the consumer of an allocation +// have their own loops over the same threads, which may not even have the same +// bounds, so an access is only meaningful alongside the loops around it. +struct ThreadLoop { + string name; + Expr min, extent; +}; + +// An access to the allocation being checked, by dimension, and the loops over +// threads it sits in, outermost first. +struct Access { + vector args; + vector thread_loops; + bool is_store; +}; + +class FindAccesses : public IRVisitor { + using IRVisitor::visit; + + void visit(const For *op) override { + if (op->for_type == ForType::GPUThread) { + thread_loops.push_back({op->name, op->min, op->extent()}); + per_thread.insert(op->name); + IRVisitor::visit(op); + thread_loops.pop_back(); + } else { + if (!thread_loops.empty()) { + per_thread.insert(op->name); + } + IRVisitor::visit(op); + } + } + + void visit(const LetStmt *op) override { + if (!thread_loops.empty()) { + per_thread.insert(op->name); + } + IRVisitor::visit(op); + } + + void visit(const Let *op) override { + if (!thread_loops.empty()) { + per_thread.insert(op->name); + } + IRVisitor::visit(op); + } + + void visit(const Provide *op) override { + if (op->name == func) { + accesses.push_back({op->args, thread_loops, true}); + } + IRVisitor::visit(op); + } + + void visit(const Call *op) override { + if (op->name == func && op->call_type == Call::Halide) { + accesses.push_back({op->args, thread_loops, false}); + } + IRVisitor::visit(op); + } + + const string &func; + vector thread_loops; + +public: + vector accesses; + // Names bound at or inside the loops over threads, so they may take a + // different value in a different thread. Everything else, such as the base + // of the block's tile, is shared by the whole block. + std::set per_thread; + + FindAccesses(const string &func) + : func(func) { + } +}; + +// Rewrite an access in terms of the loops the fused loops over threads will +// use. Counting inwards, the nth loop around an access is the nth thread +// dimension, whatever it is called, and its min is folded in, because the +// fused loop starts at zero. +vector canonical(const Access &a) { + vector args = a.args; + for (size_t i = 0; i < a.thread_loops.size() && i < 3; i++) { + const ThreadLoop &t = a.thread_loops[a.thread_loops.size() - 1 - i]; + Expr v = Variable::make(Int(32), gpu_thread_name((int)i)) + t.min; + for (Expr &arg : args) { + arg = simplify(substitute(t.name, v, arg)); + } + } + return args; +} + +// Rename only the variables that can differ between two threads of a block. +// Renaming the rest would describe a thread of some other block, which is not +// the question being asked. +class RenamePerThreadVars : public IRMutator { + using IRMutator::visit; + + Expr visit(const Variable *op) override { + if (names.count(op->name)) { + return Variable::make(op->type, op->name + "$_"); + } + return op; + } + + const std::set &names; + +public: + using IRMutator::mutate; + + RenamePerThreadVars(const std::set &names) + : names(names) { + } +}; + +class CheckCrossTalk : public IRVisitor { + using IRVisitor::visit; + + bool in_threads = false; + + void visit(const For *op) override { + if (op->for_type == ForType::GPUThread || op->for_type == ForType::GPULane) { + ScopedValue bind(in_threads, true); + IRVisitor::visit(op); + } else { + IRVisitor::visit(op); + } + } + + void visit(const Realize *op) override { + // Only memory that is private to a thread, and only when the + // allocation is outside the loops over threads. An allocation with an + // automatic memory type that lands outside them goes to shared memory, + // which the threads of a block really do share. + if (!in_threads && + (op->memory_type == MemoryType::Register || + op->memory_type == MemoryType::Stack)) { + check(op); + } + IRVisitor::visit(op); + } + + void check(const Realize *op) { + FindAccesses finder(op->name); + op->body.accept(&finder); + + int thread_dims = 0; + for (const Access &a : finder.accesses) { + thread_dims = std::max(thread_dims, (int)std::min(a.thread_loops.size(), (size_t)3)); + } + if (thread_dims == 0) { + // Only one thread runs this, so there is no one to talk to. + return; + } + + // Make an access by some other thread, in the manner of + // can_parallelize_rvar, and try to prove the two threads can never + // meet at the same site. Comparing the arguments one dimension at a + // time is what makes this provable. Anything the simplifier cannot see + // through, such as the data-dependent argument of a scatter, just + // means we fail to prove it and reject the schedule. + std::set per_thread = finder.per_thread; + for (int i = 0; i < 3; i++) { + per_thread.insert(gpu_thread_name(i)); + } + RenamePerThreadVars renamer(per_thread); + Expr distinct = const_false(); + Scope bounds; + for (int i = 0; i < thread_dims; i++) { + const string &name = gpu_thread_name(i); + Expr me = Variable::make(Int(32), name); + Expr them = Variable::make(Int(32), name + "$_"); + distinct = distinct || (me != them); + Expr extent; + for (const Access &a : finder.accesses) { + size_t n = a.thread_loops.size(); + if ((int)n > i) { + const Expr &e = a.thread_loops[n - 1 - i].extent; + extent = extent.defined() ? simplify(max(extent, e)) : e; + } + } + if (extent.defined() && is_const(extent)) { + Interval in(0, simplify(extent - 1)); + bounds.push(name, in); + bounds.push(name + "$_", in); + } + } + + // A thread may only touch what it stores itself, so look for a meeting + // between any access and some other thread's store. + Expr hazard = const_false(); + for (const Access &a : finder.accesses) { + vector mine = canonical(a); + for (const Access &b : finder.accesses) { + if (!b.is_store) { + continue; + } + vector theirs = canonical(b); + if (mine.size() != theirs.size()) { + continue; + } + Expr meet = const_true(); + for (size_t i = 0; i < mine.size(); i++) { + meet = meet && (mine[i] == renamer.mutate(theirs[i])); + } + hazard = hazard || (distinct && meet); + } + } + + hazard = common_subexpression_elimination(hazard); + hazard = substitute_in_boolean_lets(hazard); + hazard = simplify(hazard, bounds); + + if (!is_const_zero(hazard)) { + std::ostringstream accessed; + for (const Access &a : finder.accesses) { + accessed << " " << op->name << "("; + for (size_t i = 0; i < a.args.size(); i++) { + accessed << (i ? ", " : "") << a.args[i]; + } + accessed << ")" << (a.is_store ? " (stored)" : " (loaded)") << "\n"; + } + user_error + << "The allocation " << op->name << " is scheduled to live in " + << op->memory_type << " memory, which is private to a GPU thread, but it is " + << "scheduled outside the loops over GPU threads, so every thread gets its " + << "own copy of it rather than sharing one. Halide could not prove that each " + << "thread keeps to its own part of it, so a thread may be relying on a value " + << "another thread was responsible for, which it does not have. It is " + << "accessed at:\n" + << accessed.str() + << "Either schedule " << op->name << " inside the loops over GPU threads, or " + << "store it in GPUShared memory, which the threads of a block do share.\n"; + } + } +}; + +} // namespace + +void check_gpu_cross_talk(const Stmt &s) { + CheckCrossTalk checker; + s.accept(&checker); +} + +} // namespace Internal +} // namespace Halide diff --git a/src/CheckGPUCrossTalk.h b/src/CheckGPUCrossTalk.h new file mode 100644 index 000000000000..ea37dbd8a9ef --- /dev/null +++ b/src/CheckGPUCrossTalk.h @@ -0,0 +1,35 @@ +#ifndef HALIDE_CHECK_GPU_CROSS_TALK_H +#define HALIDE_CHECK_GPU_CROSS_TALK_H + +/** \file + * + * Defines a lowering pass that checks that allocations in memory private to a + * GPU thread are not shared between GPU threads. + */ + +#include "Expr.h" + +namespace Halide { +namespace Internal { + +/** Check that no allocation in memory private to a GPU thread, scheduled + * outside the loops over GPU threads, is accessed by more than one thread. + * Such an allocation gives every thread its own copy rather than one copy + * shared by the block, so a thread that touches a part of it another thread + * was responsible for gets its own copy of that part instead. Raises a user + * error if it cannot prove each thread keeps to its own part. + * + * Validation usually wants to be as early in lowering as possible, but this + * check is pinned between two passes. It must run before storage flattening, + * while the arguments of an access are still separated by dimension: once they + * have been flattened into a single index, showing that two threads cannot + * meet requires undoing the flattening, which needs modular arithmetic the + * simplifier will not do. It must run after storage folding, which rewrites + * access indices modulo a fold factor, and so can make two threads that were + * touching separate parts of an allocation touch the same part. */ +void check_gpu_cross_talk(const Stmt &s); + +} // namespace Internal +} // namespace Halide + +#endif diff --git a/src/Lower.cpp b/src/Lower.cpp index e179376e05c9..327382f296eb 100644 --- a/src/Lower.cpp +++ b/src/Lower.cpp @@ -20,6 +20,7 @@ #include "BoundsInference.h" #include "CSE.h" #include "CanonicalizeGPUVars.h" +#include "CheckGPUCrossTalk.h" #include "ClampUnsafeAccesses.h" #include "Debug.h" #include "DebugArguments.h" @@ -293,6 +294,12 @@ void lower_impl(const vector &output_funcs, s = bound_small_allocations(s); log("Lowering after bounding small realizations:", s); + // After storage folding, which can make two threads touch the same part of + // an allocation, and before storage flattening, which makes the check much + // harder. See CheckGPUCrossTalk.h. + debug(1) << "Checking for GPU cross-talk...\n"; + check_gpu_cross_talk(s); + debug(1) << "Performing storage flattening...\n"; s = storage_flattening(s, outputs, env, t); log("Lowering after storage flattening:", s); diff --git a/src/ParallelRVar.cpp b/src/ParallelRVar.cpp index 538cd144f449..7d4ac091bab0 100644 --- a/src/ParallelRVar.cpp +++ b/src/ParallelRVar.cpp @@ -49,45 +49,6 @@ class FindLoads : public IRVisitor { vector> loads; }; -/** Rename all free variables to unique new names. */ -class RenameFreeVars : public IRMutator { - using IRMutator::visit; - - map new_names; - - Expr visit(const Variable *op) override { - if (!op->param.defined() && !op->image.defined()) { - return Variable::make(op->type, get_new_name(op->name)); - } else { - return op; - } - } - -public: - string get_new_name(const string &s) { - map::iterator iter = new_names.find(s); - if (iter != new_names.end()) { - return iter->second; - } else { - string new_name = s + "$_"; - new_names[s] = new_name; - return new_name; - } - } -}; - -/** Substitute in boolean expressions. */ -class SubstituteInBooleanLets : public IRMutator { - using IRMutator::visit; - - Expr visit(const Let *op) override { - if (op->value.type() == Bool()) { - return substitute(op->name, mutate(op->value), mutate(op->body)); - } else { - return IRMutator::visit(op); - } - } -}; } // namespace bool can_parallelize_rvar(const string &v, @@ -156,7 +117,7 @@ bool can_parallelize_rvar(const string &v, debug(3) << "Attempting to falsify: " << hazard << "\n"; // Pull out common non-boolean terms hazard = common_subexpression_elimination(hazard); - hazard = SubstituteInBooleanLets()(hazard); + hazard = substitute_in_boolean_lets(hazard); hazard = simplify(hazard, bounds); debug(3) << "Simplified to: " << hazard << "\n"; diff --git a/src/Substitute.cpp b/src/Substitute.cpp index bf8822b8bcff..59288d410298 100644 --- a/src/Substitute.cpp +++ b/src/Substitute.cpp @@ -236,5 +236,43 @@ Stmt substitute_in_all_lets(const Stmt &stmt) { return SubstituteInAllLets()(stmt); } +Expr RenameFreeVars::visit(const Variable *op) { + if (!op->param.defined() && !op->image.defined()) { + return Variable::make(op->type, get_new_name(op->name)); + } else { + return op; + } +} + +const std::string &RenameFreeVars::get_new_name(const std::string &s) { + auto [it, inserted] = new_names.emplace(s, s); + if (inserted) { + it->second = s + "$_"; + } + return it->second; +} + +namespace { +class SubstituteInBooleanLets : public IRMutator { +public: + using IRMutator::mutate; + +private: + using IRMutator::visit; + + Expr visit(const Let *op) override { + if (op->value.type() == Bool()) { + return substitute(op->name, mutate(op->value), mutate(op->body)); + } else { + return IRMutator::visit(op); + } + } +}; +} // namespace + +Expr substitute_in_boolean_lets(const Expr &e) { + return SubstituteInBooleanLets().mutate(e); +} + } // namespace Internal } // namespace Halide diff --git a/src/Substitute.h b/src/Substitute.h index ae3c5f7c4d45..ee510e3c68ad 100644 --- a/src/Substitute.h +++ b/src/Substitute.h @@ -11,6 +11,7 @@ #include #include "Expr.h" +#include "IRMutator.h" namespace Halide { namespace Internal { @@ -68,6 +69,27 @@ Expr substitute_in_all_lets(const Expr &expr); Stmt substitute_in_all_lets(const Stmt &stmt); // @} +/** Rename every free variable in some IR to a fresh name, so that the result + * describes the same computation performed by a different instance of + * something: another thread, or another value of a loop variable. Ask it for + * the new name of a variable to say when the two instances differ. */ +class RenameFreeVars : public IRMutator { + using IRMutator::visit; + + std::map new_names; + + Expr visit(const Variable *op) override; + +public: + using IRMutator::mutate; + + const std::string &get_new_name(const std::string &s); +}; + +/** Substitute in any let whose value is a boolean, so that the simplifier can + * see the conditions it is being asked to reason about. */ +Expr substitute_in_boolean_lets(const Expr &e); + } // namespace Internal } // namespace Halide diff --git a/test/error/CMakeLists.txt b/test/error/CMakeLists.txt index 377f672cb0a7..e86e38b54b61 100644 --- a/test/error/CMakeLists.txt +++ b/test/error/CMakeLists.txt @@ -76,6 +76,7 @@ tests( hoist_storage_extern.cpp hoist_storage_root_without_compute_root.cpp hoist_storage_without_compute_at.cpp + gpu_register_crosstalk.cpp host_inside_gpu_loop.cpp implicit_args.cpp impossible_constraints.cpp diff --git a/test/error/gpu_register_crosstalk.cpp b/test/error/gpu_register_crosstalk.cpp new file mode 100644 index 000000000000..51aeb56b7b4e --- /dev/null +++ b/test/error/gpu_register_crosstalk.cpp @@ -0,0 +1,25 @@ +#include "Halide.h" +#include + +using namespace Halide; + +int main(int argc, char **argv) { + Func f("f"), g("g"); + Var x("x"), y("y"), xi("xi"), yi("yi"); + + f(x, y) = x + y * 1000; + // The second term reads the value the neighbouring thread computed. + g(x, y) = f(x, y) * 2 + f(x - 1, y - 1); + + g.gpu_tile(x, y, x, y, xi, yi, 16, 16); + + // f lives in registers, which are private to a thread, but it is computed + // at the block level by all the threads together, so no thread has the + // whole of it. + f.compute_at(g, x).store_in(MemoryType::Register).gpu_threads(x, y); + + g.compile_jit(Target{"host-cuda"}); + + printf("Success!\n"); + return 0; +} From 353a622219c88ace5a3d22b9baa8ef5c20aa76c6 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Thu, 6 Aug 2026 16:21:02 -0700 Subject: [PATCH 02/13] Also reject GPU register allocations only some threads store to An allocation of thread-private memory outside the loops over threads is also broken when a store to it is not in as many loops over threads as the loads are. Fusing the loops leaves such a store guarded by a test that only the first thread of the missing dimensions passes, so every other thread reads its own copy of something it never wrote. A Func computed at the block level in registers hits this, which is an ordinary thing to write. Drop the exemption for stores that do not depend on which thread they are in. The loops over threads are unordered, so it does not make a store safe, and for a store outside them it is not even true that every thread does it. Review notes: indent the new Makefile entries, have RenameFreeVars make names that are unique rather than suffixed by hand, and expand chained boolean lets in one pass rather than substituting each through the rest. Test the cases that work as well as the ones that do not: one element per thread and several per thread, against a neighbour read, a scalar stored by one thread, a row stored by one warp, and a shifted one-to-one read. Co-Authored-By: Claude Opus 5 --- Makefile | 4 +- src/CheckGPUCrossTalk.cpp | 39 ++++------- src/Substitute.cpp | 25 ++++++- src/Substitute.h | 16 ++++- test/correctness/CMakeLists.txt | 1 + .../gpu_register_at_block_level.cpp | 67 +++++++++++++++++++ test/error/CMakeLists.txt | 3 + .../gpu_register_shifted_between_threads.cpp | 21 ++++++ .../gpu_register_stored_by_one_thread.cpp | 25 +++++++ .../error/gpu_register_stored_by_one_warp.cpp | 23 +++++++ 10 files changed, 189 insertions(+), 35 deletions(-) create mode 100644 test/correctness/gpu_register_at_block_level.cpp create mode 100644 test/error/gpu_register_shifted_between_threads.cpp create mode 100644 test/error/gpu_register_stored_by_one_thread.cpp create mode 100644 test/error/gpu_register_stored_by_one_warp.cpp diff --git a/Makefile b/Makefile index bbc8a49b71fc..5435d670c42c 100644 --- a/Makefile +++ b/Makefile @@ -464,7 +464,7 @@ SOURCE_FILES = \ Buffer.cpp \ Callable.cpp \ CanonicalizeGPUVars.cpp \ -CheckGPUCrossTalk.cpp \ + CheckGPUCrossTalk.cpp \ ClampUnsafeAccesses.cpp \ Closure.cpp \ CodeGen_ARM.cpp \ @@ -668,7 +668,7 @@ HEADER_FILES = \ Buffer.h \ Callable.h \ CanonicalizeGPUVars.h \ -CheckGPUCrossTalk.h \ + CheckGPUCrossTalk.h \ ClampUnsafeAccesses.h \ Closure.h \ CodeGen_C.h \ diff --git a/src/CheckGPUCrossTalk.cpp b/src/CheckGPUCrossTalk.cpp index 6aee4fa1f0f0..7fdfc030b79c 100644 --- a/src/CheckGPUCrossTalk.cpp +++ b/src/CheckGPUCrossTalk.cpp @@ -2,6 +2,7 @@ #include "CSE.h" #include "CanonicalizeGPUVars.h" +#include "ExprUsesVar.h" #include "IR.h" #include "IROperator.h" #include "IRPrinter.h" @@ -113,29 +114,6 @@ vector canonical(const Access &a) { return args; } -// Rename only the variables that can differ between two threads of a block. -// Renaming the rest would describe a thread of some other block, which is not -// the question being asked. -class RenamePerThreadVars : public IRMutator { - using IRMutator::visit; - - Expr visit(const Variable *op) override { - if (names.count(op->name)) { - return Variable::make(op->type, op->name + "$_"); - } - return op; - } - - const std::set &names; - -public: - using IRMutator::mutate; - - RenamePerThreadVars(const std::set &names) - : names(names) { - } -}; - class CheckCrossTalk : public IRVisitor { using IRVisitor::visit; @@ -186,13 +164,16 @@ class CheckCrossTalk : public IRVisitor { for (int i = 0; i < 3; i++) { per_thread.insert(gpu_thread_name(i)); } - RenamePerThreadVars renamer(per_thread); + // Only rename what can differ between two threads of a block. Renaming + // the rest, such as the base of the block's tile, would describe a + // thread of some other block, which is not the question being asked. + RenameFreeVars renamer(per_thread); Expr distinct = const_false(); Scope bounds; for (int i = 0; i < thread_dims; i++) { const string &name = gpu_thread_name(i); Expr me = Variable::make(Int(32), name); - Expr them = Variable::make(Int(32), name + "$_"); + Expr them = Variable::make(Int(32), renamer.get_new_name(name)); distinct = distinct || (me != them); Expr extent; for (const Access &a : finder.accesses) { @@ -205,12 +186,15 @@ class CheckCrossTalk : public IRVisitor { if (extent.defined() && is_const(extent)) { Interval in(0, simplify(extent - 1)); bounds.push(name, in); - bounds.push(name + "$_", in); + bounds.push(renamer.get_new_name(name), in); } } // A thread may only touch what it stores itself, so look for a meeting - // between any access and some other thread's store. + // between any access and some other thread's store. A store outside + // the loops over threads is no exception: fusing the loops leaves it + // guarded by a test that only the first thread passes, so it is a + // value only that thread has. Expr hazard = const_false(); for (const Access &a : finder.accesses) { vector mine = canonical(a); @@ -219,6 +203,7 @@ class CheckCrossTalk : public IRVisitor { continue; } vector theirs = canonical(b); + if (mine.size() != theirs.size()) { continue; } diff --git a/src/Substitute.cpp b/src/Substitute.cpp index 59288d410298..55e858cb1ed7 100644 --- a/src/Substitute.cpp +++ b/src/Substitute.cpp @@ -1,4 +1,5 @@ #include "Substitute.h" +#include "Util.h" #include "IREquality.h" #include "IRMutator.h" #include "Scope.h" @@ -237,7 +238,8 @@ Stmt substitute_in_all_lets(const Stmt &stmt) { } Expr RenameFreeVars::visit(const Variable *op) { - if (!op->param.defined() && !op->image.defined()) { + if (!op->param.defined() && !op->image.defined() && + (only == nullptr || only->count(op->name))) { return Variable::make(op->type, get_new_name(op->name)); } else { return op; @@ -247,12 +249,19 @@ Expr RenameFreeVars::visit(const Variable *op) { const std::string &RenameFreeVars::get_new_name(const std::string &s) { auto [it, inserted] = new_names.emplace(s, s); if (inserted) { - it->second = s + "$_"; + // The '$' matters: unique_name returns its argument unchanged for a + // name it has not seen that does not look like one of its own, which + // for most names in the IR is the first call. Appending it forces the + // globally counted suffix, and so a name that is really new. + it->second = unique_name(s + "$"); } return it->second; } namespace { +// Chained boolean lets are common, so bind them in a scope and expand them +// where they are used, rather than substituting each one through the whole of +// the rest of the expression as we go. class SubstituteInBooleanLets : public IRMutator { public: using IRMutator::mutate; @@ -260,13 +269,23 @@ class SubstituteInBooleanLets : public IRMutator { private: using IRMutator::visit; + Scope bindings; + Expr visit(const Let *op) override { if (op->value.type() == Bool()) { - return substitute(op->name, mutate(op->value), mutate(op->body)); + ScopedBinding bind(bindings, op->name, mutate(op->value)); + return mutate(op->body); } else { return IRMutator::visit(op); } } + + Expr visit(const Variable *op) override { + if (const Expr *e = bindings.find(op->name)) { + return *e; + } + return op; + } }; } // namespace diff --git a/src/Substitute.h b/src/Substitute.h index ee510e3c68ad..5f88c8dd1e5f 100644 --- a/src/Substitute.h +++ b/src/Substitute.h @@ -9,6 +9,7 @@ #include #include #include +#include #include "Expr.h" #include "IRMutator.h" @@ -69,20 +70,29 @@ Expr substitute_in_all_lets(const Expr &expr); Stmt substitute_in_all_lets(const Stmt &stmt); // @} -/** Rename every free variable in some IR to a fresh name, so that the result +/** Rename free variables in some IR to fresh names, so that the result * describes the same computation performed by a different instance of - * something: another thread, or another value of a loop variable. Ask it for - * the new name of a variable to say when the two instances differ. */ + * something: another thread, or another value of a loop variable. Pass a set + * of names to rename only those, for when some of the variables mean the same + * thing to both instances. Ask get_new_name for the new name of a variable to + * say when the two instances differ; the names it makes are unique, so do not + * try to guess them. */ class RenameFreeVars : public IRMutator { using IRMutator::visit; std::map new_names; + const std::set *only = nullptr; Expr visit(const Variable *op) override; public: using IRMutator::mutate; + RenameFreeVars() = default; + explicit RenameFreeVars(const std::set &only) + : only(&only) { + } + const std::string &get_new_name(const std::string &s); }; diff --git a/test/correctness/CMakeLists.txt b/test/correctness/CMakeLists.txt index c82d5d5ca513..5ca439110784 100644 --- a/test/correctness/CMakeLists.txt +++ b/test/correctness/CMakeLists.txt @@ -175,6 +175,7 @@ tests( gpu_object_lifetime_2.cpp gpu_object_lifetime_3.cpp gpu_param_allocation.cpp + gpu_register_at_block_level.cpp gpu_reuse_shared_memory.cpp gpu_specialize.cpp gpu_store_in_register_with_no_lanes_loop.cpp diff --git a/test/correctness/gpu_register_at_block_level.cpp b/test/correctness/gpu_register_at_block_level.cpp new file mode 100644 index 000000000000..b03fae3c70a0 --- /dev/null +++ b/test/correctness/gpu_register_at_block_level.cpp @@ -0,0 +1,67 @@ +#include "Halide.h" +#include + +using namespace Halide; + +// An allocation in registers outside the loops over GPU threads gives every +// thread its own copy of it, so it is only usable if each thread keeps to its +// own part. These are the ways of doing that. + +int main(int argc, char **argv) { + Target target = get_jit_target_from_environment(); + if (!target.has_gpu_feature()) { + printf("[SKIP] No GPU target enabled.\n"); + return 0; + } + + { + // Each thread computes one element and reads back the one it computed. + Func f("f"), g("g"); + Var x("x"), y("y"), xi("xi"), yi("yi"); + f(x, y) = x + y * 1000; + g(x, y) = f(x, y) * 2; + g.gpu_tile(x, y, x, y, xi, yi, 16, 16); + f.compute_at(g, x).store_in(MemoryType::Register).gpu_threads(x, y); + + Buffer result = g.realize({256, 256}, target); + for (int y = 0; y < 256; y++) { + for (int x = 0; x < 256; x++) { + int correct = (x + y * 1000) * 2; + if (result(x, y) != correct) { + printf("one element per thread: result(%d, %d) = %d instead of %d\n", + x, y, result(x, y), correct); + return 1; + } + } + } + } + + { + // Each thread computes several elements and reads back several of + // them, which is the shape a thread holding a tile of an accumulator + // has. + Func f("f"), g("g"); + Var x("x"), y("y"), xi("xi"), yi("yi"), xii("xii"); + f(x, y) = x + y * 1000; + g(x, y) = f(x, y) + f(x, y) * 2; + g.gpu_tile(x, y, x, y, xi, yi, 32, 8) + .split(xi, xi, xii, 4) + .unroll(xii); + f.compute_at(g, xi).store_in(MemoryType::Register).unroll(x).unroll(y); + + Buffer result = g.realize({256, 256}, target); + for (int y = 0; y < 256; y++) { + for (int x = 0; x < 256; x++) { + int correct = (x + y * 1000) * 3; + if (result(x, y) != correct) { + printf("several elements per thread: result(%d, %d) = %d instead of %d\n", + x, y, result(x, y), correct); + return 1; + } + } + } + } + + printf("Success!\n"); + return 0; +} diff --git a/test/error/CMakeLists.txt b/test/error/CMakeLists.txt index e86e38b54b61..58fd66a7dd2e 100644 --- a/test/error/CMakeLists.txt +++ b/test/error/CMakeLists.txt @@ -77,6 +77,9 @@ tests( hoist_storage_root_without_compute_root.cpp hoist_storage_without_compute_at.cpp gpu_register_crosstalk.cpp + gpu_register_shifted_between_threads.cpp + gpu_register_stored_by_one_thread.cpp + gpu_register_stored_by_one_warp.cpp host_inside_gpu_loop.cpp implicit_args.cpp impossible_constraints.cpp diff --git a/test/error/gpu_register_shifted_between_threads.cpp b/test/error/gpu_register_shifted_between_threads.cpp new file mode 100644 index 000000000000..d3ec6f7845cc --- /dev/null +++ b/test/error/gpu_register_shifted_between_threads.cpp @@ -0,0 +1,21 @@ +#include "Halide.h" +#include + +using namespace Halide; + +int main(int argc, char **argv) { + Func g("g"), f("f"); + Var x("x"), y("y"), xi("xi"), yi("yi"); + + g(x, y) = x + y; + // One value of g per value of f, but not the one this thread stored. + f(x, y) = g(2 * x, 2 * y); + + f.gpu_tile(x, y, x, y, xi, yi, 16, 16); + g.compute_at(f, x).store_in(MemoryType::Register).gpu_threads(x, y); + + f.compile_jit(Target{"host-cuda"}); + + printf("Success!\n"); + return 0; +} diff --git a/test/error/gpu_register_stored_by_one_thread.cpp b/test/error/gpu_register_stored_by_one_thread.cpp new file mode 100644 index 000000000000..70e450cc4fea --- /dev/null +++ b/test/error/gpu_register_stored_by_one_thread.cpp @@ -0,0 +1,25 @@ +#include "Halide.h" +#include + +using namespace Halide; + +int main(int argc, char **argv) { + Func f("f"), g("g"); + Var x("x"), y("y"), xi("xi"), yi("yi"); + + f() = 42; + g(x, y) = f() + x; + + g.gpu_tile(x, y, x, y, xi, yi, 16, 16); + + // f is computed at the block level with no loops over threads of its own, + // so fusing the thread loops leaves its store guarded by a test that only + // the first thread passes. Every other thread would read its own copy of + // an allocation only the first thread wrote. + f.compute_at(g, x).store_in(MemoryType::Register); + + g.compile_jit(Target{"host-cuda"}); + + printf("Success!\n"); + return 0; +} diff --git a/test/error/gpu_register_stored_by_one_warp.cpp b/test/error/gpu_register_stored_by_one_warp.cpp new file mode 100644 index 000000000000..5ddcfa753bc2 --- /dev/null +++ b/test/error/gpu_register_stored_by_one_warp.cpp @@ -0,0 +1,23 @@ +#include "Halide.h" +#include + +using namespace Halide; + +int main(int argc, char **argv) { + Func g("g"), f("f"); + Var x("x"), y("y"), xi("xi"), yi("yi"); + + g(x) = x; + f(x, y) = g(x); + + f.gpu_tile(x, y, x, y, xi, yi, 16, 16); + + // g has no y, so only the threads at y == 0 store it, but every y reads + // it. Each of the others would read its own copy, which it never wrote. + g.compute_at(f, x).store_in(MemoryType::Register).gpu_threads(x); + + f.compile_jit(Target{"host-cuda"}); + + printf("Success!\n"); + return 0; +} From ad398a2cd7b29292f220f0927cc1d3bc5e160171 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Thu, 6 Aug 2026 17:21:18 -0700 Subject: [PATCH 03/13] Check that a thread reads only what it stored, not that threads are disjoint Requiring each thread to keep to a disjoint part of the allocation is stronger than it needs to be. A tail strategy has neighbouring threads recompute each other's values, so their parts overlap, and each still only reads what it wrote itself. Ask for that instead: every load must be within a region this same thread has already stored. Three things that took a test each to find. The store has to come first in the body, or the load an update definition does of the site it is about to store to matches that very store. It has to be in at least as many loops over threads, or a store done by one thread stands in for all of them. And the region is taken with the thread symbolic but the loops within a thread's part bounded: bounding the thread widens every region to cover all of them, which is what we are trying to tell apart, and leaving the inner loops unbounded cannot describe a tile. Tail strategies wrap their clamp in a likely intrinsic, which has to come off before any of that folds. Test a tile per thread walked by serial loops with such a tail, and two stages that agree, against two stages that do not. Co-Authored-By: Claude Opus 5 --- src/CheckGPUCrossTalk.cpp | 171 ++++++++++++------ .../gpu_register_at_block_level.cpp | 59 ++++++ test/error/CMakeLists.txt | 1 + test/error/gpu_register_stages_disagree.cpp | 25 +++ 4 files changed, 199 insertions(+), 57 deletions(-) create mode 100644 test/error/gpu_register_stages_disagree.cpp diff --git a/src/CheckGPUCrossTalk.cpp b/src/CheckGPUCrossTalk.cpp index 7fdfc030b79c..83f73005b662 100644 --- a/src/CheckGPUCrossTalk.cpp +++ b/src/CheckGPUCrossTalk.cpp @@ -1,5 +1,6 @@ #include "CheckGPUCrossTalk.h" +#include "Bounds.h" #include "CSE.h" #include "CanonicalizeGPUVars.h" #include "ExprUsesVar.h" @@ -36,11 +37,34 @@ struct Access { vector args; vector thread_loops; bool is_store; + // Where this sits in the body, so we can tell a store that has already + // happened from one that has not happened yet. + int order; }; class FindAccesses : public IRVisitor { using IRVisitor::visit; + // An index is usually in terms of let-bound variables, and the producer + // and the consumer name theirs differently, so put them back. + Expr resolve(Expr e) const { + for (auto it = lets.rbegin(); it != lets.rend(); it++) { + if (expr_uses_var(e, it->first)) { + e = substitute(it->first, it->second, e); + } + } + return e; + } + + vector resolve(const vector &args) const { + vector result; + result.reserve(args.size()); + for (const Expr &e : args) { + result.push_back(resolve(e)); + } + return result; + } + void visit(const For *op) override { if (op->for_type == ForType::GPUThread) { thread_loops.push_back({op->name, op->min, op->extent()}); @@ -51,6 +75,13 @@ class FindAccesses : public IRVisitor { if (!thread_loops.empty()) { per_thread.insert(op->name); } + // The loops a thread runs inside its own part of the allocation + // bound how far its accesses reach, which is what says the parts + // do not overlap. + Expr min = resolve(op->min), max = resolve(op->max); + if (is_const(min) && is_const(max)) { + loop_bounds.emplace_back(op->name, Interval(min, max)); + } IRVisitor::visit(op); } } @@ -59,32 +90,42 @@ class FindAccesses : public IRVisitor { if (!thread_loops.empty()) { per_thread.insert(op->name); } - IRVisitor::visit(op); + op->value.accept(this); + lets.emplace_back(op->name, op->value); + op->body.accept(this); + lets.pop_back(); } void visit(const Let *op) override { if (!thread_loops.empty()) { per_thread.insert(op->name); } - IRVisitor::visit(op); + op->value.accept(this); + lets.emplace_back(op->name, op->value); + op->body.accept(this); + lets.pop_back(); } void visit(const Provide *op) override { + // The values are read before the store happens, which matters for an + // update definition, where the value reads the site being stored to. + IRVisitor::visit(op); if (op->name == func) { - accesses.push_back({op->args, thread_loops, true}); + accesses.push_back({resolve(op->args), thread_loops, true, order++}); } - IRVisitor::visit(op); } void visit(const Call *op) override { if (op->name == func && op->call_type == Call::Halide) { - accesses.push_back({op->args, thread_loops, false}); + accesses.push_back({resolve(op->args), thread_loops, false, order++}); } IRVisitor::visit(op); } const string &func; vector thread_loops; + vector> lets; + int order = 0; public: vector accesses; @@ -92,12 +133,23 @@ class FindAccesses : public IRVisitor { // different value in a different thread. Everything else, such as the base // of the block's tile, is shared by the whole block. std::set per_thread; + vector> loop_bounds; FindAccesses(const string &func) : func(func) { } }; +string name_and_args(const string &name, const vector &args) { + std::ostringstream s; + s << name << "("; + for (size_t i = 0; i < args.size(); i++) { + s << (i ? ", " : "") << args[i]; + } + s << ")"; + return s.str(); +} + // Rewrite an access in terms of the loops the fused loops over threads will // use. Counting inwards, the nth loop around an access is the nth thread // dimension, whatever it is called, and its min is folded in, because the @@ -154,27 +206,27 @@ class CheckCrossTalk : public IRVisitor { return; } - // Make an access by some other thread, in the manner of - // can_parallelize_rvar, and try to prove the two threads can never - // meet at the same site. Comparing the arguments one dimension at a - // time is what makes this provable. Anything the simplifier cannot see - // through, such as the data-dependent argument of a scatter, just - // means we fail to prove it and reject the schedule. - std::set per_thread = finder.per_thread; - for (int i = 0; i < 3; i++) { - per_thread.insert(gpu_thread_name(i)); - } - // Only rename what can differ between two threads of a block. Renaming - // the rest, such as the base of the block's tile, would describe a - // thread of some other block, which is not the question being asked. - RenameFreeVars renamer(per_thread); - Expr distinct = const_false(); + // A thread's own copy only holds what that thread put there, so + // every load has to be of something this same thread already stored. + // Comparing the arguments one dimension at a time is what makes this + // provable; the flattened index of the same access would not be. + // + // Two threads storing to one site is not itself a problem. A tail + // strategy makes neighbouring threads recompute the same values, and + // each still reads what it wrote. + // The region a thread touches is in terms of the loops it runs + // inside its own part, so those are bounded. The thread itself is left + // symbolic: bounding it would widen every region to cover all threads, + // which is what we are trying to tell apart. Scope bounds; + for (const auto &b : finder.loop_bounds) { + bounds.push(b.first, b.second); + } + // How many threads there are is still worth knowing, to settle the + // clamp a tail strategy puts on the last thread's part. That only + // simplifies the expression; it does not go into the region. + Scope thread_bounds; for (int i = 0; i < thread_dims; i++) { - const string &name = gpu_thread_name(i); - Expr me = Variable::make(Int(32), name); - Expr them = Variable::make(Int(32), renamer.get_new_name(name)); - distinct = distinct || (me != them); Expr extent; for (const Access &a : finder.accesses) { size_t n = a.thread_loops.size(); @@ -184,49 +236,54 @@ class CheckCrossTalk : public IRVisitor { } } if (extent.defined() && is_const(extent)) { - Interval in(0, simplify(extent - 1)); - bounds.push(name, in); - bounds.push(renamer.get_new_name(name), in); + thread_bounds.push(gpu_thread_name(i), Interval(0, simplify(extent - 1))); } } - // A thread may only touch what it stores itself, so look for a meeting - // between any access and some other thread's store. A store outside - // the loops over threads is no exception: fusing the loops leaves it - // guarded by a test that only the first thread passes, so it is a - // value only that thread has. - Expr hazard = const_false(); - for (const Access &a : finder.accesses) { - vector mine = canonical(a); - for (const Access &b : finder.accesses) { - if (!b.is_store) { - continue; - } - vector theirs = canonical(b); + // A tail strategy wraps the clamp on the last thread's part in a + // likely intrinsic, which stops the simplifier folding it away once + // the number of threads is known. + auto region = [&](const Access &a, size_t dim) { + Expr e = simplify(remove_likelies(canonical(a)[dim]), thread_bounds); + return bounds_of_expr_in_scope(e, bounds); + }; - if (mine.size() != theirs.size()) { + for (const Access &load : finder.accesses) { + if (load.is_store) { + continue; + } + bool ok = false; + for (const Access &store : finder.accesses) { + // The store has to have happened already, and has to be in at + // least as many loops over threads, or it is the work of one + // thread standing in for all of them. + if (!store.is_store || + store.order > load.order || + store.thread_loops.size() < load.thread_loops.size() || + store.args.size() != load.args.size()) { continue; } - Expr meet = const_true(); - for (size_t i = 0; i < mine.size(); i++) { - meet = meet && (mine[i] == renamer.mutate(theirs[i])); + bool covers = true; + for (size_t i = 0; i < load.args.size() && covers; i++) { + Interval l = region(load, i), st = region(store, i); + covers = (l.has_lower_bound() && l.has_upper_bound() && + st.has_lower_bound() && st.has_upper_bound() && + can_prove(st.min <= l.min && l.max <= st.max)); } - hazard = hazard || (distinct && meet); + ok = ok || covers; + } + if (!ok) { + report(op, finder.accesses, load); } } + } - hazard = common_subexpression_elimination(hazard); - hazard = substitute_in_boolean_lets(hazard); - hazard = simplify(hazard, bounds); + void report(const Realize *op, const vector &accesses, const Access &load) { - if (!is_const_zero(hazard)) { std::ostringstream accessed; - for (const Access &a : finder.accesses) { - accessed << " " << op->name << "("; - for (size_t i = 0; i < a.args.size(); i++) { - accessed << (i ? ", " : "") << a.args[i]; - } - accessed << ")" << (a.is_store ? " (stored)" : " (loaded)") << "\n"; + for (const Access &a : accesses) { + accessed << " " << name_and_args(op->name, a.args) + << (a.is_store ? " (stored)" : " (loaded)") << "\n"; } user_error << "The allocation " << op->name << " is scheduled to live in " @@ -235,11 +292,11 @@ class CheckCrossTalk : public IRVisitor { << "own copy of it rather than sharing one. Halide could not prove that each " << "thread keeps to its own part of it, so a thread may be relying on a value " << "another thread was responsible for, which it does not have. It is " - << "accessed at:\n" + << "loaded at:\n " << name_and_args(op->name, load.args) + << "\nwhich is not within what this thread stores. It is accessed at:\n" << accessed.str() << "Either schedule " << op->name << " inside the loops over GPU threads, or " << "store it in GPUShared memory, which the threads of a block do share.\n"; - } } }; diff --git a/test/correctness/gpu_register_at_block_level.cpp b/test/correctness/gpu_register_at_block_level.cpp index b03fae3c70a0..3ba8f423a804 100644 --- a/test/correctness/gpu_register_at_block_level.cpp +++ b/test/correctness/gpu_register_at_block_level.cpp @@ -62,6 +62,65 @@ int main(int argc, char **argv) { } } + { + // A tile per thread, walked by serial loops rather than unrolled ones, + // with a tail strategy that lets neighbouring tiles overlap. Threads + // recompute each other's values but each still reads only its own. + Func f("f"), g("g"); + Var x("x"), y("y"), xo("xo"), yo("yo"), xi("xi"), yi("yi"), xii("xii"), yii("yii"); + Var fxo("fxo"), fyo("fyo"), fxi("fxi"), fyi("fyi"); + f(x, y) = x + y * 1000; + g(x, y) = f(x, y) * 2; + g.split(x, xo, xi, 32) + .split(y, yo, yi, 32) + .split(xi, xi, xii, 2) + .split(yi, yi, yii, 2) + .reorder(xii, yii, xi, yi, xo, yo) + .gpu_blocks(xo, yo) + .gpu_threads(xi, yi); + f.compute_at(g, xo) + .store_in(MemoryType::Register) + .split(x, fxo, fxi, 2) + .split(y, fyo, fyi, 2) + .reorder(fxi, fyi, fxo, fyo) + .gpu_threads(fxo, fyo); + + Buffer result = g.realize({128, 128}, target); + for (int y = 0; y < 128; y++) { + for (int x = 0; x < 128; x++) { + int correct = (x + y * 1000) * 2; + if (result(x, y) != correct) { + printf("tile per thread: result(%d, %d) = %d instead of %d\n", + x, y, result(x, y), correct); + return 1; + } + } + } + } + + { + // Two stages that agree about which thread owns which site. + Func f("f"), g("g"); + Var x("x"), y("y"), xi("xi"), yi("yi"); + f(x, y) = x + y; + f(x, y) += x + y; + g(x, y) = f(x, y); + g.gpu_tile(x, y, x, y, xi, yi, 16, 16); + f.compute_at(g, x).store_in(MemoryType::Register).gpu_threads(x, y); + f.update().gpu_threads(x, y); + + Buffer result = g.realize({64, 64}, target); + for (int y = 0; y < 64; y++) { + for (int x = 0; x < 64; x++) { + if (result(x, y) != 2 * (x + y)) { + printf("two stages: result(%d, %d) = %d instead of %d\n", + x, y, result(x, y), 2 * (x + y)); + return 1; + } + } + } + } + printf("Success!\n"); return 0; } diff --git a/test/error/CMakeLists.txt b/test/error/CMakeLists.txt index 58fd66a7dd2e..ee62a87e1b9c 100644 --- a/test/error/CMakeLists.txt +++ b/test/error/CMakeLists.txt @@ -78,6 +78,7 @@ tests( hoist_storage_without_compute_at.cpp gpu_register_crosstalk.cpp gpu_register_shifted_between_threads.cpp + gpu_register_stages_disagree.cpp gpu_register_stored_by_one_thread.cpp gpu_register_stored_by_one_warp.cpp host_inside_gpu_loop.cpp diff --git a/test/error/gpu_register_stages_disagree.cpp b/test/error/gpu_register_stages_disagree.cpp new file mode 100644 index 000000000000..2ccafc93f000 --- /dev/null +++ b/test/error/gpu_register_stages_disagree.cpp @@ -0,0 +1,25 @@ +#include "Halide.h" +#include + +using namespace Halide; + +int main(int argc, char **argv) { + Func f("f"), g("g"); + Var x("x"), y("y"), xi("xi"), yi("yi"); + + f(x, y) = x + y; + f(x, y) += x + y; + g(x, y) = f(x, y); + + g.gpu_tile(x, y, x, y, xi, yi, 16, 16); + f.compute_at(g, x).store_in(MemoryType::Register).gpu_threads(x, y); + + // The update maps threads to sites transposed relative to the pure + // definition, so it reads sites a different thread initialised. + f.update().reorder(y, x).gpu_threads(y, x); + + g.compile_jit(Target{"host-cuda"}); + + printf("Success!\n"); + return 0; +} From 0e0028d46c135f342d9e2365a6133658e481817a Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Fri, 7 Aug 2026 16:08:40 -0700 Subject: [PATCH 04/13] Keep loop bounds that speak of the thread when checking for cross talk A load has to be shown to be within the region the same thread stores, and the loops a thread runs inside its own part of the allocation are what bound how far its accesses reach. Only constant bounds were kept, so a loop that starts at the thread's own part of the allocation was dropped, its region came back unbounded, and the check failed. That happens as soon as anything is staged through a wrapper computed inside the thread loops. Keep the bounds whatever they are, and canonicalize them the way the accesses themselves are canonicalized, so that a bound naming one loop over threads and an access naming another are talking about the same thread. Co-Authored-By: Claude Opus 5 --- src/CheckGPUCrossTalk.cpp | 39 +++++++++++-------- .../gpu_register_at_block_level.cpp | 37 ++++++++++++++++++ 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/CheckGPUCrossTalk.cpp b/src/CheckGPUCrossTalk.cpp index 83f73005b662..b7627aaf3d2f 100644 --- a/src/CheckGPUCrossTalk.cpp +++ b/src/CheckGPUCrossTalk.cpp @@ -42,6 +42,19 @@ struct Access { int order; }; +// Rewrite an expr in terms of the loops the fused loops over threads will use. +// Counting inwards, the nth loop around it is the nth thread dimension, +// whatever it is called, and its min is folded in, because the fused loop +// starts at zero. +Expr canonicalize(Expr e, const vector &thread_loops) { + for (size_t i = 0; i < thread_loops.size() && i < 3; i++) { + const ThreadLoop &t = thread_loops[thread_loops.size() - 1 - i]; + Expr v = Variable::make(Int(32), gpu_thread_name((int)i)) + t.min; + e = simplify(substitute(t.name, v, e)); + } + return e; +} + class FindAccesses : public IRVisitor { using IRVisitor::visit; @@ -77,11 +90,12 @@ class FindAccesses : public IRVisitor { } // The loops a thread runs inside its own part of the allocation // bound how far its accesses reach, which is what says the parts - // do not overlap. - Expr min = resolve(op->min), max = resolve(op->max); - if (is_const(min) && is_const(max)) { - loop_bounds.emplace_back(op->name, Interval(min, max)); - } + // do not overlap. A loop that starts at the thread's own part of + // the allocation has bounds that speak of the thread, so they get + // the same treatment as the accesses themselves. + Expr min = canonicalize(resolve(op->min), thread_loops); + Expr max = canonicalize(resolve(op->max), thread_loops); + loop_bounds.emplace_back(op->name, Interval(min, max)); IRVisitor::visit(op); } } @@ -150,18 +164,11 @@ string name_and_args(const string &name, const vector &args) { return s.str(); } -// Rewrite an access in terms of the loops the fused loops over threads will -// use. Counting inwards, the nth loop around an access is the nth thread -// dimension, whatever it is called, and its min is folded in, because the -// fused loop starts at zero. vector canonical(const Access &a) { - vector args = a.args; - for (size_t i = 0; i < a.thread_loops.size() && i < 3; i++) { - const ThreadLoop &t = a.thread_loops[a.thread_loops.size() - 1 - i]; - Expr v = Variable::make(Int(32), gpu_thread_name((int)i)) + t.min; - for (Expr &arg : args) { - arg = simplify(substitute(t.name, v, arg)); - } + vector args; + args.reserve(a.args.size()); + for (const Expr &arg : a.args) { + args.push_back(canonicalize(arg, a.thread_loops)); } return args; } diff --git a/test/correctness/gpu_register_at_block_level.cpp b/test/correctness/gpu_register_at_block_level.cpp index 3ba8f423a804..12ed2c467039 100644 --- a/test/correctness/gpu_register_at_block_level.cpp +++ b/test/correctness/gpu_register_at_block_level.cpp @@ -121,6 +121,43 @@ int main(int argc, char **argv) { } } + { + // A tile per thread, read back through a wrapper computed inside the + // loops over threads. The wrapper's own loops start at the thread's + // part of the allocation, so their bounds speak of the thread rather + // than being constants, and they still have to bound how far it + // reaches. + Func f("f"), g("g"); + Var x("x"), y("y"), xi("xi"), yi("yi"), xii("xii"), yii("yii"); + f(x, y) = x + y * 100; + g(x, y) = f(x, y) * 2; + g.tile(x, y, xi, yi, 32, 16) + .tile(xi, yi, xii, yii, 2, 2) + .gpu_blocks(x, y) + .gpu_threads(xi, yi) + .unroll(xii) + .unroll(yii); + f.compute_at(g, x) + .store_in(MemoryType::Register) + .tile(x, y, xii, yii, 2, 2) + .gpu_threads(x, y) + .unroll(xii) + .unroll(yii); + f.in().compute_at(g, xi).unroll(x).unroll(y); + + Buffer result = g.realize({64, 64}, target); + for (int y = 0; y < 64; y++) { + for (int x = 0; x < 64; x++) { + int correct = (x + y * 100) * 2; + if (result(x, y) != correct) { + printf("wrapper: result(%d, %d) = %d instead of %d\n", + x, y, result(x, y), correct); + return 1; + } + } + } + } + printf("Success!\n"); return 0; } From dcd03117c9ca6ea8585c1f7eee3f65efb33379f6 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Mon, 10 Aug 2026 14:27:35 -0700 Subject: [PATCH 05/13] Simplify the cross-talk check The check was reformulated twice, and each formulation left something behind. The first one proved that no two threads could meet, which meant renaming one thread's variables to make a second instance of it, so the machinery for that was hoisted out of ParallelRVar.cpp. Reading a thread's own stores instead needs none of it, and nothing but ParallelRVar.cpp has used it since. Put those files back the way they were. Along the same lines, drop per_thread, which nothing reads, and the includes and helper that went with the formulations that are gone. An access no longer carries its own copy of the loops over threads it sits in. Its arguments are put in terms of the fused loops when it is recorded, which is all anything downstream wanted, along with how deep it sits and how many threads there are. Its position in the list is what says whether it has already happened, so it needs no separate ordering. Each access's region is now computed once rather than once per candidate partner, which is the difference between one simplifier pass per access per dimension and one per pair of accesses per dimension. Note in both places that decide what a loop over threads is why a loop over lanes counts for one and not the other: the lanes of a warp share registers, which is what makes warp shuffles work, but two threads share nothing. Co-Authored-By: Claude Opus 5 --- src/CheckGPUCrossTalk.cpp | 176 ++++++++++++++++++++------------------ src/ParallelRVar.cpp | 41 ++++++++- src/Substitute.cpp | 57 ------------ src/Substitute.h | 32 ------- 4 files changed, 131 insertions(+), 175 deletions(-) diff --git a/src/CheckGPUCrossTalk.cpp b/src/CheckGPUCrossTalk.cpp index b7627aaf3d2f..d5b105b24bbb 100644 --- a/src/CheckGPUCrossTalk.cpp +++ b/src/CheckGPUCrossTalk.cpp @@ -1,18 +1,15 @@ #include "CheckGPUCrossTalk.h" #include "Bounds.h" -#include "CSE.h" #include "CanonicalizeGPUVars.h" #include "ExprUsesVar.h" #include "IR.h" #include "IROperator.h" #include "IRPrinter.h" -#include "IRMutator.h" #include "IRVisitor.h" #include "Simplify.h" #include "Substitute.h" -#include #include namespace Halide { @@ -31,15 +28,18 @@ struct ThreadLoop { Expr min, extent; }; -// An access to the allocation being checked, by dimension, and the loops over -// threads it sits in, outermost first. +// An access to the allocation being checked, by dimension. Accesses are +// recorded in the order they appear, so an earlier one in the list is one that +// has already happened. struct Access { + // As written, for error messages. vector args; - vector thread_loops; + // In terms of the loops the fused loops over threads will use, so that two + // accesses can be compared. + vector canonical_args; + // How many loops over threads this sits in. + int thread_depth; bool is_store; - // Where this sits in the body, so we can tell a store that has already - // happened from one that has not happened yet. - int order; }; // Rewrite an expr in terms of the loops the fused loops over threads will use. @@ -79,15 +79,15 @@ class FindAccesses : public IRVisitor { } void visit(const For *op) override { + // Only a loop over threads separates one thread's copy of the + // allocation from another's. A loop over lanes does not: the lanes of a + // warp can reach each other's registers, which is what LowerWarpShuffles + // is for, so one is just another loop a single thread runs. if (op->for_type == ForType::GPUThread) { thread_loops.push_back({op->name, op->min, op->extent()}); - per_thread.insert(op->name); IRVisitor::visit(op); thread_loops.pop_back(); } else { - if (!thread_loops.empty()) { - per_thread.insert(op->name); - } // The loops a thread runs inside its own part of the allocation // bound how far its accesses reach, which is what says the parts // do not overlap. A loop that starts at the thread's own part of @@ -101,9 +101,6 @@ class FindAccesses : public IRVisitor { } void visit(const LetStmt *op) override { - if (!thread_loops.empty()) { - per_thread.insert(op->name); - } op->value.accept(this); lets.emplace_back(op->name, op->value); op->body.accept(this); @@ -111,27 +108,44 @@ class FindAccesses : public IRVisitor { } void visit(const Let *op) override { - if (!thread_loops.empty()) { - per_thread.insert(op->name); - } op->value.accept(this); lets.emplace_back(op->name, op->value); op->body.accept(this); lets.pop_back(); } + // Record an access, in terms of both the loops it was written with and the + // loops over threads it will end up in. + void record(const vector &args, bool is_store) { + vector resolved = resolve(args), canonical; + canonical.reserve(resolved.size()); + for (const Expr &e : resolved) { + canonical.push_back(canonicalize(e, thread_loops)); + } + // The nth loop counting inwards from this access is the nth thread + // dimension, so that is where its extent belongs. + int depth = (int)thread_loops.size(); + for (int i = 0; i < depth && i < 3; i++) { + const Expr &e = thread_loops[depth - 1 - i].extent; + thread_extents[i] = thread_extents[i].defined() ? + simplify(max(thread_extents[i], e)) : + e; + } + accesses.push_back({resolved, canonical, depth, is_store}); + } + void visit(const Provide *op) override { // The values are read before the store happens, which matters for an // update definition, where the value reads the site being stored to. IRVisitor::visit(op); if (op->name == func) { - accesses.push_back({resolve(op->args), thread_loops, true, order++}); + record(op->args, true); } } void visit(const Call *op) override { if (op->name == func && op->call_type == Call::Halide) { - accesses.push_back({resolve(op->args), thread_loops, false, order++}); + record(op->args, false); } IRVisitor::visit(op); } @@ -139,15 +153,12 @@ class FindAccesses : public IRVisitor { const string &func; vector thread_loops; vector> lets; - int order = 0; public: vector accesses; - // Names bound at or inside the loops over threads, so they may take a - // different value in a different thread. Everything else, such as the base - // of the block's tile, is shared by the whole block. - std::set per_thread; vector> loop_bounds; + // The most threads there are in each dimension, if that is known. + Expr thread_extents[3]; FindAccesses(const string &func) : func(func) { @@ -164,21 +175,17 @@ string name_and_args(const string &name, const vector &args) { return s.str(); } -vector canonical(const Access &a) { - vector args; - args.reserve(a.args.size()); - for (const Expr &arg : a.args) { - args.push_back(canonicalize(arg, a.thread_loops)); - } - return args; -} - class CheckCrossTalk : public IRVisitor { using IRVisitor::visit; bool in_threads = false; void visit(const For *op) override { + // An allocation inside a loop over threads or lanes already belongs to + // whoever runs that loop, so there is nothing to tell apart. Note that + // a lane loop counts here but not when deciding which loops separate + // one thread's copy from another's, because a warp shares its lanes' + // registers but two threads share nothing. if (op->for_type == ForType::GPUThread || op->for_type == ForType::GPULane) { ScopedValue bind(in_threads, true); IRVisitor::visit(op); @@ -206,7 +213,10 @@ class CheckCrossTalk : public IRVisitor { int thread_dims = 0; for (const Access &a : finder.accesses) { - thread_dims = std::max(thread_dims, (int)std::min(a.thread_loops.size(), (size_t)3)); + thread_dims = std::max(thread_dims, std::min(a.thread_depth, 3)); + internal_assert(a.args.size() == finder.accesses[0].args.size()) + << "Accesses to " << op->name << " disagree about how many " + << "dimensions it has\n"; } if (thread_dims == 0) { // Only one thread runs this, so there is no one to talk to. @@ -234,50 +244,47 @@ class CheckCrossTalk : public IRVisitor { // simplifies the expression; it does not go into the region. Scope thread_bounds; for (int i = 0; i < thread_dims; i++) { - Expr extent; - for (const Access &a : finder.accesses) { - size_t n = a.thread_loops.size(); - if ((int)n > i) { - const Expr &e = a.thread_loops[n - 1 - i].extent; - extent = extent.defined() ? simplify(max(extent, e)) : e; - } - } + const Expr &extent = finder.thread_extents[i]; if (extent.defined() && is_const(extent)) { thread_bounds.push(gpu_thread_name(i), Interval(0, simplify(extent - 1))); } } - // A tail strategy wraps the clamp on the last thread's part in a - // likely intrinsic, which stops the simplifier folding it away once - // the number of threads is known. - auto region = [&](const Access &a, size_t dim) { - Expr e = simplify(remove_likelies(canonical(a)[dim]), thread_bounds); - return bounds_of_expr_in_scope(e, bounds); - }; + // The region each access touches, by dimension. A tail strategy wraps + // the clamp on the last thread's part in a likely intrinsic, which + // stops the simplifier folding it away once the number of threads is + // known. + vector> regions(finder.accesses.size()); + for (size_t i = 0; i < finder.accesses.size(); i++) { + for (const Expr &arg : finder.accesses[i].canonical_args) { + Expr e = simplify(remove_likelies(arg), thread_bounds); + regions[i].push_back(bounds_of_expr_in_scope(e, bounds)); + } + } - for (const Access &load : finder.accesses) { + for (size_t l = 0; l < finder.accesses.size(); l++) { + const Access &load = finder.accesses[l]; if (load.is_store) { continue; } bool ok = false; - for (const Access &store : finder.accesses) { - // The store has to have happened already, and has to be in at - // least as many loops over threads, or it is the work of one - // thread standing in for all of them. - if (!store.is_store || - store.order > load.order || - store.thread_loops.size() < load.thread_loops.size() || - store.args.size() != load.args.size()) { + // Stores later in the list have definitely not happened yet. Ones + // earlier have, unless the two sit in different arms of the same + // if, which is not accounted for here. + for (size_t s = 0; s < l && !ok; s++) { + const Access &store = finder.accesses[s]; + // The store has to be in at least as many loops over threads, + // or it is the work of one thread standing in for all of them. + if (!store.is_store || store.thread_depth < load.thread_depth) { continue; } - bool covers = true; - for (size_t i = 0; i < load.args.size() && covers; i++) { - Interval l = region(load, i), st = region(store, i); - covers = (l.has_lower_bound() && l.has_upper_bound() && - st.has_lower_bound() && st.has_upper_bound() && - can_prove(st.min <= l.min && l.max <= st.max)); + ok = true; + for (size_t i = 0; i < regions[l].size() && ok; i++) { + const Interval &want = regions[l][i], &have = regions[s][i]; + ok = (want.has_lower_bound() && want.has_upper_bound() && + have.has_lower_bound() && have.has_upper_bound() && + can_prove(have.min <= want.min && want.max <= have.max)); } - ok = ok || covers; } if (!ok) { report(op, finder.accesses, load); @@ -286,24 +293,23 @@ class CheckCrossTalk : public IRVisitor { } void report(const Realize *op, const vector &accesses, const Access &load) { - - std::ostringstream accessed; - for (const Access &a : accesses) { - accessed << " " << name_and_args(op->name, a.args) - << (a.is_store ? " (stored)" : " (loaded)") << "\n"; - } - user_error - << "The allocation " << op->name << " is scheduled to live in " - << op->memory_type << " memory, which is private to a GPU thread, but it is " - << "scheduled outside the loops over GPU threads, so every thread gets its " - << "own copy of it rather than sharing one. Halide could not prove that each " - << "thread keeps to its own part of it, so a thread may be relying on a value " - << "another thread was responsible for, which it does not have. It is " - << "loaded at:\n " << name_and_args(op->name, load.args) - << "\nwhich is not within what this thread stores. It is accessed at:\n" - << accessed.str() - << "Either schedule " << op->name << " inside the loops over GPU threads, or " - << "store it in GPUShared memory, which the threads of a block do share.\n"; + std::ostringstream accessed; + for (const Access &a : accesses) { + accessed << " " << name_and_args(op->name, a.args) + << (a.is_store ? " (stored)" : " (loaded)") << "\n"; + } + user_error + << "The allocation " << op->name << " is scheduled to live in " + << op->memory_type << " memory, which is private to a GPU thread, but it is " + << "scheduled outside the loops over GPU threads, so every thread gets its " + << "own copy of it rather than sharing one. Halide could not prove that each " + << "thread keeps to its own part of it, so a thread may be relying on a value " + << "another thread was responsible for, which it does not have. It is " + << "loaded at:\n " << name_and_args(op->name, load.args) + << "\nwhich is not within what this thread stores. It is accessed at:\n" + << accessed.str() + << "Either schedule " << op->name << " inside the loops over GPU threads, or " + << "store it in GPUShared memory, which the threads of a block do share.\n"; } }; diff --git a/src/ParallelRVar.cpp b/src/ParallelRVar.cpp index 7d4ac091bab0..538cd144f449 100644 --- a/src/ParallelRVar.cpp +++ b/src/ParallelRVar.cpp @@ -49,6 +49,45 @@ class FindLoads : public IRVisitor { vector> loads; }; +/** Rename all free variables to unique new names. */ +class RenameFreeVars : public IRMutator { + using IRMutator::visit; + + map new_names; + + Expr visit(const Variable *op) override { + if (!op->param.defined() && !op->image.defined()) { + return Variable::make(op->type, get_new_name(op->name)); + } else { + return op; + } + } + +public: + string get_new_name(const string &s) { + map::iterator iter = new_names.find(s); + if (iter != new_names.end()) { + return iter->second; + } else { + string new_name = s + "$_"; + new_names[s] = new_name; + return new_name; + } + } +}; + +/** Substitute in boolean expressions. */ +class SubstituteInBooleanLets : public IRMutator { + using IRMutator::visit; + + Expr visit(const Let *op) override { + if (op->value.type() == Bool()) { + return substitute(op->name, mutate(op->value), mutate(op->body)); + } else { + return IRMutator::visit(op); + } + } +}; } // namespace bool can_parallelize_rvar(const string &v, @@ -117,7 +156,7 @@ bool can_parallelize_rvar(const string &v, debug(3) << "Attempting to falsify: " << hazard << "\n"; // Pull out common non-boolean terms hazard = common_subexpression_elimination(hazard); - hazard = substitute_in_boolean_lets(hazard); + hazard = SubstituteInBooleanLets()(hazard); hazard = simplify(hazard, bounds); debug(3) << "Simplified to: " << hazard << "\n"; diff --git a/src/Substitute.cpp b/src/Substitute.cpp index 55e858cb1ed7..bf8822b8bcff 100644 --- a/src/Substitute.cpp +++ b/src/Substitute.cpp @@ -1,5 +1,4 @@ #include "Substitute.h" -#include "Util.h" #include "IREquality.h" #include "IRMutator.h" #include "Scope.h" @@ -237,61 +236,5 @@ Stmt substitute_in_all_lets(const Stmt &stmt) { return SubstituteInAllLets()(stmt); } -Expr RenameFreeVars::visit(const Variable *op) { - if (!op->param.defined() && !op->image.defined() && - (only == nullptr || only->count(op->name))) { - return Variable::make(op->type, get_new_name(op->name)); - } else { - return op; - } -} - -const std::string &RenameFreeVars::get_new_name(const std::string &s) { - auto [it, inserted] = new_names.emplace(s, s); - if (inserted) { - // The '$' matters: unique_name returns its argument unchanged for a - // name it has not seen that does not look like one of its own, which - // for most names in the IR is the first call. Appending it forces the - // globally counted suffix, and so a name that is really new. - it->second = unique_name(s + "$"); - } - return it->second; -} - -namespace { -// Chained boolean lets are common, so bind them in a scope and expand them -// where they are used, rather than substituting each one through the whole of -// the rest of the expression as we go. -class SubstituteInBooleanLets : public IRMutator { -public: - using IRMutator::mutate; - -private: - using IRMutator::visit; - - Scope bindings; - - Expr visit(const Let *op) override { - if (op->value.type() == Bool()) { - ScopedBinding bind(bindings, op->name, mutate(op->value)); - return mutate(op->body); - } else { - return IRMutator::visit(op); - } - } - - Expr visit(const Variable *op) override { - if (const Expr *e = bindings.find(op->name)) { - return *e; - } - return op; - } -}; -} // namespace - -Expr substitute_in_boolean_lets(const Expr &e) { - return SubstituteInBooleanLets().mutate(e); -} - } // namespace Internal } // namespace Halide diff --git a/src/Substitute.h b/src/Substitute.h index 5f88c8dd1e5f..ae3c5f7c4d45 100644 --- a/src/Substitute.h +++ b/src/Substitute.h @@ -9,10 +9,8 @@ #include #include #include -#include #include "Expr.h" -#include "IRMutator.h" namespace Halide { namespace Internal { @@ -70,36 +68,6 @@ Expr substitute_in_all_lets(const Expr &expr); Stmt substitute_in_all_lets(const Stmt &stmt); // @} -/** Rename free variables in some IR to fresh names, so that the result - * describes the same computation performed by a different instance of - * something: another thread, or another value of a loop variable. Pass a set - * of names to rename only those, for when some of the variables mean the same - * thing to both instances. Ask get_new_name for the new name of a variable to - * say when the two instances differ; the names it makes are unique, so do not - * try to guess them. */ -class RenameFreeVars : public IRMutator { - using IRMutator::visit; - - std::map new_names; - const std::set *only = nullptr; - - Expr visit(const Variable *op) override; - -public: - using IRMutator::mutate; - - RenameFreeVars() = default; - explicit RenameFreeVars(const std::set &only) - : only(&only) { - } - - const std::string &get_new_name(const std::string &s); -}; - -/** Substitute in any let whose value is a boolean, so that the simplifier can - * see the conditions it is being asked to reason about. */ -Expr substitute_in_boolean_lets(const Expr &e); - } // namespace Internal } // namespace Halide From 0cce81d92e659078dc000590b5bd72fc1f0dade4 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Tue, 11 Aug 2026 09:45:37 -0700 Subject: [PATCH 06/13] Note why an if doesn't break the program-order test Co-Authored-By: Claude Opus 5 --- src/CheckGPUCrossTalk.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/CheckGPUCrossTalk.cpp b/src/CheckGPUCrossTalk.cpp index d5b105b24bbb..af357aa21081 100644 --- a/src/CheckGPUCrossTalk.cpp +++ b/src/CheckGPUCrossTalk.cpp @@ -268,9 +268,12 @@ class CheckCrossTalk : public IRVisitor { continue; } bool ok = false; - // Stores later in the list have definitely not happened yet. Ones - // earlier have, unless the two sit in different arms of the same - // if, which is not accounted for here. + // Stores later in the list have not happened yet. Ones earlier + // have, except across the arms of an if, which doesn't matter here: + // any value the output depends on was stored by some thread, so if + // no other thread stored this one, this thread did. A site this + // thread never wrote holds a value nothing depends on, like the + // garbage that pads out a vector. for (size_t s = 0; s < l && !ok; s++) { const Access &store = finder.accesses[s]; // The store has to be in at least as many loops over threads, From 23bbbffa319501d173761094ae02520ccc420562 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Tue, 11 Aug 2026 10:01:49 -0700 Subject: [PATCH 07/13] Wrap an access's lets around it instead of substituting them in Substituting copies each let's value into every use, so a chain of lets that each build on the one before expands into something the size of their product. Wrap the definitions around the index instead. Everything the index is then put through - substitute, simplify, and bounds_of_expr_in_scope - already handles lets. The one place that wants them expanded is the description of an access in an error message, which only happens on the way to aborting. Co-Authored-By: Claude Opus 5 --- src/CheckGPUCrossTalk.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/CheckGPUCrossTalk.cpp b/src/CheckGPUCrossTalk.cpp index af357aa21081..a5167c76ba09 100644 --- a/src/CheckGPUCrossTalk.cpp +++ b/src/CheckGPUCrossTalk.cpp @@ -59,11 +59,14 @@ class FindAccesses : public IRVisitor { using IRVisitor::visit; // An index is usually in terms of let-bound variables, and the producer - // and the consumer name theirs differently, so put them back. + // and the consumer name theirs differently, so put the definitions back + // around it. Wrapping rather than substituting keeps a chain of lets that + // each use the one before it from expanding into something the size of + // their product. Expr resolve(Expr e) const { for (auto it = lets.rbegin(); it != lets.rend(); it++) { if (expr_uses_var(e, it->first)) { - e = substitute(it->first, it->second, e); + e = Let::make(it->first, it->second, e); } } return e; @@ -165,11 +168,13 @@ class FindAccesses : public IRVisitor { } }; +// Only used to describe an access in an error message, so this is the one +// place the lets are worth expanding, however large that gets. string name_and_args(const string &name, const vector &args) { std::ostringstream s; s << name << "("; for (size_t i = 0; i < args.size(); i++) { - s << (i ? ", " : "") << args[i]; + s << (i ? ", " : "") << substitute_in_all_lets(args[i]); } s << ")"; return s.str(); From 74b574f63b595679b751bdfb07ff280606a73357 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Wed, 12 Aug 2026 12:46:43 -0700 Subject: [PATCH 08/13] Use the shared let helpers, and show accesses as written resolve() rewrapped the peeled lets by hand, testing each one against the body with expr_uses_var, which is quadratic in the number of lets. rewrap_used_lets does the same thing in one pass. An access in an error message is now printed the way it was written rather than with its lets expanded. Expanding them produces a bigger expression, not a clearer one, and the names it drops are the ones the schedule used. Co-Authored-By: Claude Opus 5 --- src/CheckGPUCrossTalk.cpp | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/src/CheckGPUCrossTalk.cpp b/src/CheckGPUCrossTalk.cpp index a5167c76ba09..8d4b8fdba839 100644 --- a/src/CheckGPUCrossTalk.cpp +++ b/src/CheckGPUCrossTalk.cpp @@ -32,7 +32,8 @@ struct ThreadLoop { // recorded in the order they appear, so an earlier one in the list is one that // has already happened. struct Access { - // As written, for error messages. + // As written, for error messages. Left in terms of whatever variables the + // schedule named, which is what the user will recognize. vector args; // In terms of the loops the fused loops over threads will use, so that two // accesses can be compared. @@ -60,16 +61,9 @@ class FindAccesses : public IRVisitor { // An index is usually in terms of let-bound variables, and the producer // and the consumer name theirs differently, so put the definitions back - // around it. Wrapping rather than substituting keeps a chain of lets that - // each use the one before it from expanding into something the size of - // their product. - Expr resolve(Expr e) const { - for (auto it = lets.rbegin(); it != lets.rend(); it++) { - if (expr_uses_var(e, it->first)) { - e = Let::make(it->first, it->second, e); - } - } - return e; + // around it. + Expr resolve(const Expr &e) const { + return rewrap_used_lets(e, lets); } vector resolve(const vector &args) const { @@ -120,10 +114,10 @@ class FindAccesses : public IRVisitor { // Record an access, in terms of both the loops it was written with and the // loops over threads it will end up in. void record(const vector &args, bool is_store) { - vector resolved = resolve(args), canonical; - canonical.reserve(resolved.size()); - for (const Expr &e : resolved) { - canonical.push_back(canonicalize(e, thread_loops)); + vector canonical; + canonical.reserve(args.size()); + for (const Expr &e : args) { + canonical.push_back(canonicalize(resolve(e), thread_loops)); } // The nth loop counting inwards from this access is the nth thread // dimension, so that is where its extent belongs. @@ -134,7 +128,7 @@ class FindAccesses : public IRVisitor { simplify(max(thread_extents[i], e)) : e; } - accesses.push_back({resolved, canonical, depth, is_store}); + accesses.push_back({args, canonical, depth, is_store}); } void visit(const Provide *op) override { @@ -168,13 +162,11 @@ class FindAccesses : public IRVisitor { } }; -// Only used to describe an access in an error message, so this is the one -// place the lets are worth expanding, however large that gets. string name_and_args(const string &name, const vector &args) { std::ostringstream s; s << name << "("; for (size_t i = 0; i < args.size(); i++) { - s << (i ? ", " : "") << substitute_in_all_lets(args[i]); + s << (i ? ", " : "") << args[i]; } s << ")"; return s.str(); From 56c355916f4ee8ea51b7d917d3a4aac1e3926e71 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Wed, 12 Aug 2026 13:05:02 -0700 Subject: [PATCH 09/13] Fold the let rewrapping into canonicalize Every place that put a let-bound index back together went on to rewrite it in terms of the fused loops over threads, so canonicalize does both, and as a member it needs neither the lets nor the loops passed to it. Co-Authored-By: Claude Opus 5 --- src/CheckGPUCrossTalk.cpp | 44 +++++++++++++-------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/src/CheckGPUCrossTalk.cpp b/src/CheckGPUCrossTalk.cpp index 8d4b8fdba839..73f2a4ecfb6f 100644 --- a/src/CheckGPUCrossTalk.cpp +++ b/src/CheckGPUCrossTalk.cpp @@ -2,7 +2,6 @@ #include "Bounds.h" #include "CanonicalizeGPUVars.h" -#include "ExprUsesVar.h" #include "IR.h" #include "IROperator.h" #include "IRPrinter.h" @@ -43,34 +42,22 @@ struct Access { bool is_store; }; -// Rewrite an expr in terms of the loops the fused loops over threads will use. -// Counting inwards, the nth loop around it is the nth thread dimension, -// whatever it is called, and its min is folded in, because the fused loop -// starts at zero. -Expr canonicalize(Expr e, const vector &thread_loops) { - for (size_t i = 0; i < thread_loops.size() && i < 3; i++) { - const ThreadLoop &t = thread_loops[thread_loops.size() - 1 - i]; - Expr v = Variable::make(Int(32), gpu_thread_name((int)i)) + t.min; - e = simplify(substitute(t.name, v, e)); - } - return e; -} - class FindAccesses : public IRVisitor { using IRVisitor::visit; - // An index is usually in terms of let-bound variables, and the producer - // and the consumer name theirs differently, so put the definitions back - // around it. - Expr resolve(const Expr &e) const { - return rewrap_used_lets(e, lets); - } - - vector resolve(const vector &args) const { - vector result; - result.reserve(args.size()); - for (const Expr &e : args) { - result.push_back(resolve(e)); + // Rewrite an expr into the terms two accesses can be compared in. An index + // is usually written using let-bound variables, and the producer and the + // consumer name theirs differently, so put the definitions back around it. + // Then swap each loop over threads for the fused loop it will become: + // counting inwards, the nth loop around the expr is the nth thread + // dimension, whatever it is called, and its min is folded in, because the + // fused loop starts at zero. + Expr canonicalize(const Expr &e) const { + Expr result = rewrap_used_lets(e, lets); + for (size_t i = 0; i < thread_loops.size() && i < 3; i++) { + const ThreadLoop &t = thread_loops[thread_loops.size() - 1 - i]; + Expr v = Variable::make(Int(32), gpu_thread_name((int)i)) + t.min; + result = simplify(substitute(t.name, v, result)); } return result; } @@ -90,8 +77,7 @@ class FindAccesses : public IRVisitor { // do not overlap. A loop that starts at the thread's own part of // the allocation has bounds that speak of the thread, so they get // the same treatment as the accesses themselves. - Expr min = canonicalize(resolve(op->min), thread_loops); - Expr max = canonicalize(resolve(op->max), thread_loops); + Expr min = canonicalize(op->min), max = canonicalize(op->max); loop_bounds.emplace_back(op->name, Interval(min, max)); IRVisitor::visit(op); } @@ -117,7 +103,7 @@ class FindAccesses : public IRVisitor { vector canonical; canonical.reserve(args.size()); for (const Expr &e : args) { - canonical.push_back(canonicalize(resolve(e), thread_loops)); + canonical.push_back(canonicalize(e)); } // The nth loop counting inwards from this access is the nth thread // dimension, so that is where its extent belongs. From 46fcb3a958611ba10b0a45c611c937f258b79c45 Mon Sep 17 00:00:00 2001 From: "halide-ci[bot]" <266445882+halide-ci[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:53:54 +0000 Subject: [PATCH 10/13] Apply pre-commit auto-fixes --- test/error/CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/error/CMakeLists.txt b/test/error/CMakeLists.txt index ee62a87e1b9c..5ff9f97d944d 100644 --- a/test/error/CMakeLists.txt +++ b/test/error/CMakeLists.txt @@ -73,14 +73,14 @@ tests( func_tuple_update_types_mismatch.cpp fuse_same_var.cpp fuse_vectorized_var_with_rvar.cpp - hoist_storage_extern.cpp - hoist_storage_root_without_compute_root.cpp - hoist_storage_without_compute_at.cpp gpu_register_crosstalk.cpp gpu_register_shifted_between_threads.cpp gpu_register_stages_disagree.cpp gpu_register_stored_by_one_thread.cpp gpu_register_stored_by_one_warp.cpp + hoist_storage_extern.cpp + hoist_storage_root_without_compute_root.cpp + hoist_storage_without_compute_at.cpp host_inside_gpu_loop.cpp implicit_args.cpp impossible_constraints.cpp From fb7d5a1b42181a9be982753fa949138a0d8a91af Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Fri, 14 Aug 2026 09:23:05 -0700 Subject: [PATCH 11/13] Test the cross-talk check on whatever GPU is around The check is not specific to CUDA, so the tests for it shouldn't be either. Use the GPU the environment names and skip when it names none, the way other GPU tests do. Verified that all five still report the error under OpenCL as well as CUDA, and that the one that should compile still does. Co-Authored-By: Claude Opus 5 --- test/error/gpu_register_crosstalk.cpp | 8 +++++++- test/error/gpu_register_shifted_between_threads.cpp | 8 +++++++- test/error/gpu_register_stages_disagree.cpp | 8 +++++++- test/error/gpu_register_stored_by_one_thread.cpp | 8 +++++++- test/error/gpu_register_stored_by_one_warp.cpp | 8 +++++++- 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/test/error/gpu_register_crosstalk.cpp b/test/error/gpu_register_crosstalk.cpp index 51aeb56b7b4e..aa863f2d73ba 100644 --- a/test/error/gpu_register_crosstalk.cpp +++ b/test/error/gpu_register_crosstalk.cpp @@ -4,6 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + Target target = get_jit_target_from_environment(); + if (!target.has_gpu_feature()) { + printf("[SKIP] No GPU target enabled.\n"); + return 0; + } + Func f("f"), g("g"); Var x("x"), y("y"), xi("xi"), yi("yi"); @@ -18,7 +24,7 @@ int main(int argc, char **argv) { // whole of it. f.compute_at(g, x).store_in(MemoryType::Register).gpu_threads(x, y); - g.compile_jit(Target{"host-cuda"}); + g.compile_jit(target); printf("Success!\n"); return 0; diff --git a/test/error/gpu_register_shifted_between_threads.cpp b/test/error/gpu_register_shifted_between_threads.cpp index d3ec6f7845cc..2031c2de00aa 100644 --- a/test/error/gpu_register_shifted_between_threads.cpp +++ b/test/error/gpu_register_shifted_between_threads.cpp @@ -4,6 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + Target target = get_jit_target_from_environment(); + if (!target.has_gpu_feature()) { + printf("[SKIP] No GPU target enabled.\n"); + return 0; + } + Func g("g"), f("f"); Var x("x"), y("y"), xi("xi"), yi("yi"); @@ -14,7 +20,7 @@ int main(int argc, char **argv) { f.gpu_tile(x, y, x, y, xi, yi, 16, 16); g.compute_at(f, x).store_in(MemoryType::Register).gpu_threads(x, y); - f.compile_jit(Target{"host-cuda"}); + f.compile_jit(target); printf("Success!\n"); return 0; diff --git a/test/error/gpu_register_stages_disagree.cpp b/test/error/gpu_register_stages_disagree.cpp index 2ccafc93f000..4e61a271a5fb 100644 --- a/test/error/gpu_register_stages_disagree.cpp +++ b/test/error/gpu_register_stages_disagree.cpp @@ -4,6 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + Target target = get_jit_target_from_environment(); + if (!target.has_gpu_feature()) { + printf("[SKIP] No GPU target enabled.\n"); + return 0; + } + Func f("f"), g("g"); Var x("x"), y("y"), xi("xi"), yi("yi"); @@ -18,7 +24,7 @@ int main(int argc, char **argv) { // definition, so it reads sites a different thread initialised. f.update().reorder(y, x).gpu_threads(y, x); - g.compile_jit(Target{"host-cuda"}); + g.compile_jit(target); printf("Success!\n"); return 0; diff --git a/test/error/gpu_register_stored_by_one_thread.cpp b/test/error/gpu_register_stored_by_one_thread.cpp index 70e450cc4fea..61527f8249e2 100644 --- a/test/error/gpu_register_stored_by_one_thread.cpp +++ b/test/error/gpu_register_stored_by_one_thread.cpp @@ -4,6 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + Target target = get_jit_target_from_environment(); + if (!target.has_gpu_feature()) { + printf("[SKIP] No GPU target enabled.\n"); + return 0; + } + Func f("f"), g("g"); Var x("x"), y("y"), xi("xi"), yi("yi"); @@ -18,7 +24,7 @@ int main(int argc, char **argv) { // an allocation only the first thread wrote. f.compute_at(g, x).store_in(MemoryType::Register); - g.compile_jit(Target{"host-cuda"}); + g.compile_jit(target); printf("Success!\n"); return 0; diff --git a/test/error/gpu_register_stored_by_one_warp.cpp b/test/error/gpu_register_stored_by_one_warp.cpp index 5ddcfa753bc2..86563460c840 100644 --- a/test/error/gpu_register_stored_by_one_warp.cpp +++ b/test/error/gpu_register_stored_by_one_warp.cpp @@ -4,6 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + Target target = get_jit_target_from_environment(); + if (!target.has_gpu_feature()) { + printf("[SKIP] No GPU target enabled.\n"); + return 0; + } + Func g("g"), f("f"); Var x("x"), y("y"), xi("xi"), yi("yi"); @@ -16,7 +22,7 @@ int main(int argc, char **argv) { // it. Each of the others would read its own copy, which it never wrote. g.compute_at(f, x).store_in(MemoryType::Register).gpu_threads(x); - f.compile_jit(Target{"host-cuda"}); + f.compile_jit(target); printf("Success!\n"); return 0; From 0219e29c1608e3326f4799f87c41bb7cb9f64769 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Sun, 16 Aug 2026 13:46:44 -0700 Subject: [PATCH 12/13] Don't skip the register cross-talk tests without a GPU An error test that skips produces no error, which the Makefile's test harness reads as a failure. These are compile-time checks, so they need a GPU API but not a GPU: name one when the environment doesn't, and the check gets tested everywhere rather than only where a GPU is attached. Co-Authored-By: Claude Opus 5 --- test/error/gpu_register_crosstalk.cpp | 6 ++++-- test/error/gpu_register_shifted_between_threads.cpp | 6 ++++-- test/error/gpu_register_stages_disagree.cpp | 6 ++++-- test/error/gpu_register_stored_by_one_thread.cpp | 6 ++++-- test/error/gpu_register_stored_by_one_warp.cpp | 6 ++++-- 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/test/error/gpu_register_crosstalk.cpp b/test/error/gpu_register_crosstalk.cpp index aa863f2d73ba..597e0b478a94 100644 --- a/test/error/gpu_register_crosstalk.cpp +++ b/test/error/gpu_register_crosstalk.cpp @@ -4,10 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + // The check happens when the pipeline is compiled, so it needs a GPU API + // but not a GPU. Use the one the environment names, and pick one if it + // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - printf("[SKIP] No GPU target enabled.\n"); - return 0; + target.set_feature(Target::CUDA); } Func f("f"), g("g"); diff --git a/test/error/gpu_register_shifted_between_threads.cpp b/test/error/gpu_register_shifted_between_threads.cpp index 2031c2de00aa..86bcd5cd9341 100644 --- a/test/error/gpu_register_shifted_between_threads.cpp +++ b/test/error/gpu_register_shifted_between_threads.cpp @@ -4,10 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + // The check happens when the pipeline is compiled, so it needs a GPU API + // but not a GPU. Use the one the environment names, and pick one if it + // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - printf("[SKIP] No GPU target enabled.\n"); - return 0; + target.set_feature(Target::CUDA); } Func g("g"), f("f"); diff --git a/test/error/gpu_register_stages_disagree.cpp b/test/error/gpu_register_stages_disagree.cpp index 4e61a271a5fb..ef0db76f71e9 100644 --- a/test/error/gpu_register_stages_disagree.cpp +++ b/test/error/gpu_register_stages_disagree.cpp @@ -4,10 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + // The check happens when the pipeline is compiled, so it needs a GPU API + // but not a GPU. Use the one the environment names, and pick one if it + // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - printf("[SKIP] No GPU target enabled.\n"); - return 0; + target.set_feature(Target::CUDA); } Func f("f"), g("g"); diff --git a/test/error/gpu_register_stored_by_one_thread.cpp b/test/error/gpu_register_stored_by_one_thread.cpp index 61527f8249e2..c08cc6fb3683 100644 --- a/test/error/gpu_register_stored_by_one_thread.cpp +++ b/test/error/gpu_register_stored_by_one_thread.cpp @@ -4,10 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + // The check happens when the pipeline is compiled, so it needs a GPU API + // but not a GPU. Use the one the environment names, and pick one if it + // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - printf("[SKIP] No GPU target enabled.\n"); - return 0; + target.set_feature(Target::CUDA); } Func f("f"), g("g"); diff --git a/test/error/gpu_register_stored_by_one_warp.cpp b/test/error/gpu_register_stored_by_one_warp.cpp index 86563460c840..aa10869fc426 100644 --- a/test/error/gpu_register_stored_by_one_warp.cpp +++ b/test/error/gpu_register_stored_by_one_warp.cpp @@ -4,10 +4,12 @@ using namespace Halide; int main(int argc, char **argv) { + // The check happens when the pipeline is compiled, so it needs a GPU API + // but not a GPU. Use the one the environment names, and pick one if it + // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - printf("[SKIP] No GPU target enabled.\n"); - return 0; + target.set_feature(Target::CUDA); } Func g("g"), f("f"); From 74fa54bf0a32c300f27653daa5c10c09042d5520 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Sun, 16 Aug 2026 14:18:57 -0700 Subject: [PATCH 13/13] Skip the register cross-talk tests without a GPU, but still error An error test that returns cleanly produces no error, which the Makefile's test harness reads as a failure. Skip the way the other error tests that can't always run do, with an assert to report something. Co-Authored-By: Claude Opus 5 --- test/error/gpu_register_crosstalk.cpp | 7 +++---- test/error/gpu_register_shifted_between_threads.cpp | 7 +++---- test/error/gpu_register_stages_disagree.cpp | 7 +++---- test/error/gpu_register_stored_by_one_thread.cpp | 7 +++---- test/error/gpu_register_stored_by_one_warp.cpp | 7 +++---- 5 files changed, 15 insertions(+), 20 deletions(-) diff --git a/test/error/gpu_register_crosstalk.cpp b/test/error/gpu_register_crosstalk.cpp index 597e0b478a94..df6513003655 100644 --- a/test/error/gpu_register_crosstalk.cpp +++ b/test/error/gpu_register_crosstalk.cpp @@ -4,12 +4,11 @@ using namespace Halide; int main(int argc, char **argv) { - // The check happens when the pipeline is compiled, so it needs a GPU API - // but not a GPU. Use the one the environment names, and pick one if it - // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - target.set_feature(Target::CUDA); + printf("[SKIP] No GPU target enabled.\n"); + // An error test has to report an error even when it skips. + _halide_user_assert(0); } Func f("f"), g("g"); diff --git a/test/error/gpu_register_shifted_between_threads.cpp b/test/error/gpu_register_shifted_between_threads.cpp index 86bcd5cd9341..60602268a8a4 100644 --- a/test/error/gpu_register_shifted_between_threads.cpp +++ b/test/error/gpu_register_shifted_between_threads.cpp @@ -4,12 +4,11 @@ using namespace Halide; int main(int argc, char **argv) { - // The check happens when the pipeline is compiled, so it needs a GPU API - // but not a GPU. Use the one the environment names, and pick one if it - // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - target.set_feature(Target::CUDA); + printf("[SKIP] No GPU target enabled.\n"); + // An error test has to report an error even when it skips. + _halide_user_assert(0); } Func g("g"), f("f"); diff --git a/test/error/gpu_register_stages_disagree.cpp b/test/error/gpu_register_stages_disagree.cpp index ef0db76f71e9..dc6f76e3672f 100644 --- a/test/error/gpu_register_stages_disagree.cpp +++ b/test/error/gpu_register_stages_disagree.cpp @@ -4,12 +4,11 @@ using namespace Halide; int main(int argc, char **argv) { - // The check happens when the pipeline is compiled, so it needs a GPU API - // but not a GPU. Use the one the environment names, and pick one if it - // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - target.set_feature(Target::CUDA); + printf("[SKIP] No GPU target enabled.\n"); + // An error test has to report an error even when it skips. + _halide_user_assert(0); } Func f("f"), g("g"); diff --git a/test/error/gpu_register_stored_by_one_thread.cpp b/test/error/gpu_register_stored_by_one_thread.cpp index c08cc6fb3683..f103b2a7ee9b 100644 --- a/test/error/gpu_register_stored_by_one_thread.cpp +++ b/test/error/gpu_register_stored_by_one_thread.cpp @@ -4,12 +4,11 @@ using namespace Halide; int main(int argc, char **argv) { - // The check happens when the pipeline is compiled, so it needs a GPU API - // but not a GPU. Use the one the environment names, and pick one if it - // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - target.set_feature(Target::CUDA); + printf("[SKIP] No GPU target enabled.\n"); + // An error test has to report an error even when it skips. + _halide_user_assert(0); } Func f("f"), g("g"); diff --git a/test/error/gpu_register_stored_by_one_warp.cpp b/test/error/gpu_register_stored_by_one_warp.cpp index aa10869fc426..19d44c4bcc4b 100644 --- a/test/error/gpu_register_stored_by_one_warp.cpp +++ b/test/error/gpu_register_stored_by_one_warp.cpp @@ -4,12 +4,11 @@ using namespace Halide; int main(int argc, char **argv) { - // The check happens when the pipeline is compiled, so it needs a GPU API - // but not a GPU. Use the one the environment names, and pick one if it - // names none, so that this is still tested on a machine without a GPU. Target target = get_jit_target_from_environment(); if (!target.has_gpu_feature()) { - target.set_feature(Target::CUDA); + printf("[SKIP] No GPU target enabled.\n"); + // An error test has to report an error even when it skips. + _halide_user_assert(0); } Func g("g"), f("f");