Add struct types to Halide - #9416
alexreinking wants to merge 8 commits into
Conversation
9f99483 to
069ce04
Compare
069ce04 to
50683f7
Compare
50683f7 to
e9123a1
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9416 +/- ##
==========================================
- Coverage 70.11% 70.11% -0.01%
==========================================
Files 261 262 +1
Lines 79940 80508 +568
Branches 19478 19637 +159
==========================================
+ Hits 56052 56450 +398
- Misses 18047 18173 +126
- Partials 5841 5885 +44 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
e9123a1 to
425cb95
Compare
425cb95 to
2740239
Compare
1464bc0 to
168ce01
Compare
168ce01 to
e272037
Compare
166d1d5 to
1444b20
Compare
1444b20 to
91c252b
Compare
91c252b to
14ba508
Compare
Introduce first-class packed struct types (Type::Struct) modeled faithfully in the type system: - A dedicated ABI type code halide_type_struct=5; a struct's packed byte size rides in the halide_type_t reserved field, so struct-typed buffers have correct element size/strides. StructTypeInfo (field layout) is interned like handle metadata, keeping Type at 8 bytes. is_uint()/etc. are honestly false for structs, so no numeric special-casing is needed. - field()/pack_struct() intrinsics with byte-addressed lowering (LowerStructTypes), plus per-field pack_struct ergonomics: an array field is filled by a gather() packet, a gather(extent, gen) generator, a single expression with one swept `_` placeholder (index arithmetic allowed), or a field() copy of a whole same-typed field. - Python bindings for the type, field/pack_struct/gather, and the per-field forms. - Tests: correctness (CPU + GPU), error cases, Python, and an ARM codegen test implementing ggml's q4_0/q8_0 dot product that verifies the packed qs arrays lower to dense 128-bit vector loads. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- ConstantBounds.cpp/PyType.cpp: drop two unnecessary-copy clang-tidy findings.
- struct_type_dot_product.cpp: back the test buffers with 16-byte-aligned
storage instead of std::vector, whose allocator doesn't guarantee that on
32-bit ABIs.
- FuseGPUThreadLoops.cpp: relax the GPU shared/global allocation clustering
assert from "both types are powers of two bytes" to the actual requirement
("widest type is a whole multiple of the cluster's byte-granularity type"),
so a non-power-of-two struct size (e.g. 12 bytes) can share GPU memory with
other types.
- LowerStructTypes.cpp: build a packed float field (e.g. a struct's fp16
delta) via a scalar shift/or chain instead of concat_bits's
vector-shuffle-then-reinterpret lowering. Some AArch64 backends (LLVM < 23)
can't legalize a bitcast straight from a vector to a scalar half, and any
chain of pure bitcasts collapses back to that illegal form during
optimization -- only avoiding the vector shape in the first place works.
- struct_type.cpp: move constant_bounds_test() (dead code -- declared and
defined but never called since Halide's old internal-test runner was
removed) into a proper correctness test.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- CodeGen_D3D12Compute_Dev: struct-backed groupshared/local arrays are
stored as raw bytes, but Load/Store still consulted the original
Type::Struct(...) for cast/promotion bookkeeping, crashing print_cast's
internal_assert(source_type.is_uint()) since a struct is neither int,
uint, nor float. Treat the storage element as Int(8) (matching what
print_type_maybe_storage actually emits) and scale the groupshared
array's declared element count by the struct's byte size.
- CodeGen_Vulkan_Dev: visit(Allocate) passed Type::Struct(...) directly
to SpvBuilder::declare_type, which has no notion of struct types
("SPIRV: Unsupported type"). Use UInt(8) as the element type for
declaration and later Load/Store bookkeeping instead.
- Serialization/Deserialization: add a Struct TypeCode and StructField
table to the flatbuffers schema so struct types (and their field
layout) round-trip instead of being rejected outright. This is needed
because CI runs JIT compiles through a serialize/deserialize
round-trip for regression testing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
129cd6a to
53dcddc
Compare
| // direct syntactic access to the underlying struct_pack()/Select/Load. Because | ||
| // struct-typed values are never materialized (only individual fields are), | ||
| // inlining it back in at each field() use site costs nothing at runtime. | ||
| Scope<Expr> struct_lets; |
There was a problem hiding this comment.
I think this risks forwarding lets that contain loads across stores that clobber them, e.g. in the obscure case that you have an update def on a Tuple-valued Func where one tuple component is a struct and there is cross-talk between tuple elements. A potential solution discussed in person is turning this:
let foo = some struct
...
... foo.field1 ... foo.field2 ...
into this:
let foo_field1 = lowered rhs of the original let for field1
let foo_field2 = lowered rhs of the original let for field2
...
... foo_field1 ... foo_field2 ...
| make_const(index_type, field_base_offset) + | ||
| cast(index_type, elem_index) * make_const(index_type, elem_bytes); | ||
|
|
||
| auto byte_load = [&](int b) { |
There was a problem hiding this comment.
It would be simpler (if the vectorization pass allows it) to lower it to a single load of a byte vector + a reinterpret to a scalar. In the common case of a struct size and field offset that are a multiple of the scalar size, it could just be a flat load of the scalar type.
| << op->name << " was requested this way.\n"; | ||
| int extent = f.array_extent.value_or(1); | ||
| int elem_bytes = f.type.bytes(); | ||
| for (int e = 0; e < extent; e++) { |
There was a problem hiding this comment.
Comments analogous to my load ones apply here. The stores could be byte-vector stores for unaligned or scalar stores for aligned fields, instead of storing one byte at a time.
| std::string name; | ||
| Type type; | ||
| std::optional<int> array_extent = std::nullopt; | ||
|
|
There was a problem hiding this comment.
Suggest a bool aligned, which is true when its offset in the containing struct is a multiple of its scalar size and the containing struct's total size is also a multiple of its scalar size. This could also be computed as needed at the usage site, or as a helper method on StructTypeInfo if the same StructField can be used in multiple StructTypeInfos
| uint8_t bits; | ||
|
|
||
| /** Reserved for future element-kind payloads. */ | ||
| /** Element-kind payload. For a struct type (code == halide_type_struct) |
There was a problem hiding this comment.
Given that it's no longer reserved for future use, I recommend changing the name, e.g. to "info"
| * packed byte size -- the buffer only carries the ABI halide_type_t, which | ||
| * forgets a struct's field layout (like a buffer of handles forgets its | ||
| * pointee type), so two structs of equal size are ABI-compatible. */ | ||
| bool is_compatible_for_buffer_bind(const Type &buffer_type) const { |
There was a problem hiding this comment.
Suggested alternative name: matches_buffer_abi_type
| test_runtime_index_into_inlined_pack(); | ||
| test_pack_struct_field_copy(); | ||
| printf("Success!\n"); | ||
| return 0; |
There was a problem hiding this comment.
Please add a tuple-of-structs test where there's an update definition that involves cross-talk between tuple elements (as a regression test for the potential issue pointed out above)
| // block_q4_0 { fp16 d; uint8 qs[16]; } // 32 quants, 2 nibbles per byte | ||
| // block_q8_0 { fp16 d; int8 qs[32]; } // 32 quants | ||
| // | ||
| // This mirrors ggml_vec_dot_q4_0_q8_0 (ggml-cpu/arch/arm/quants.c): for each |
There was a problem hiding this comment.
This refers directly to a path from another PR
There was a problem hiding this comment.
Oops, misundersood. Please add a github link or something for ggml
This PR adds struct types to Halide. This allows for inputs and outputs to be defined in terms of popular wire formats, even if they aren't prepared for efficient computation. For example, you can define GGML's
q5_0andq8_0formats like so:Type q5_0 = Type::Struct({{"d", Float(16)}, {"qh", UInt(32)}, {"qs", UInt(8), 16}}); Type q8_0 = Type::Struct({{"d", Float(16)}, {"qs", Int(8), 32}});Then you can write a pipeline that consumes them:
ImageParam x{q4_0_type(), 1, "x"}; ImageParam y{q8_0_type(), 1, "y"}; Var b("b"), k("k"), u("u"); RDom r(0, x.dim(0).extent(), 0, 32, "r"); // block, quant // Dequantize one 4-bit weight to float: quant k is a nibble of the // packed byte k % 16 -- its low nibble for k < 16, its high nibble // otherwise -- biased by -8 to signed [-8, 7], times the block delta. Expr nib = field(x(b), "qs")[k % 16]; // uint8 Func x_wt("x_wt"); x_wt(k, b) = cast<float>(field(x(b), "d")) * (cast<int32_t>(select(k < 16, nib % 16, nib / 16)) - 8); // Dequantize one int8 activation to float: quant k is int8 k, times the // block delta. Func y_wt("y_wt"); y_wt(k, b) = cast<float>(field(y(b), "d")) * cast<int32_t>(field(y(b), "qs")[k]); // Dequantize each side and sum every product. Func qdot{"qdot"}; qdot() = 0.0f; qdot() += x_wt(r[1], r[0]) * y_wt(r[1], r[0]);As you can see, struct types support both individual elements, as well as fixed-size array elements. Fields are accessed with a named
field(expr, "name")intrinsic. The tests include additional cases for struct-typed fields and output structs.This adjusts the
Typeandhalide_type_trepresentations in the following ways:Type: likeHandle, aStructtype has a compile-time known field layout.halide_type_t: structs get their own kind, and the reserved field records the number of bytes in the struct.All layouts are expected to be packed. No padding is inserted around or in between field elements.
Breaking changes
None—it's a new feature.
Checklist