Repository navigation
Conversation
|
I’ll start resolving the merge conflicts and update the cuTile implementation to use the new tilelens. |
Performance Benchmark
Iterations: 1 warmup + 20 measured |
| run: | | ||
| cd tilelens | ||
| uv sync --extra test | ||
| uv sync --extra test --extra cutile |
There was a problem hiding this comment.
cutile can be a default dependency
| <h1>Analyze and debug your tile-based DSL kernels</h1> | ||
| <p style="display: block; font-size: 0.95rem; color: var(--text-muted); margin-top: -1rem; margin-bottom: 2.5rem;"> | ||
| Note: Triton is the default DSL frontend; Amazon NKI support is optional and in active development. | ||
| Note: Triton is the default DSL frontend; Amazon NKI and NVIDIA cuTile support is in active development. |
There was a problem hiding this comment.
We could make cuTile default if you're confident
mark14wu
left a comment
There was a problem hiding this comment.
Thanks for the PR. The integration through the shared frontend adapters works well, CuTileTrace follows the same pattern as NKI, and the cuTile tests pass locally.
I reviewed this mainly by differential testing against real cuTile:
- cuda-tile 1.5.0 with
[tileiras]on an RTX 4090: I launched the same kernels viact.launchand compared result dtypes and values with the interpreter. - I ran all 45 TileBench cuTile operators through the interpreter and compared them against both the GPU run and the torch references.
Each inline comment includes a repro. Summary:
Must fix
- The example kernel and two test kernels do not compile on real cuTile (
math.ceilin the loop bound,ct.arangecalled withoutdtype). - bf16
ct.sum/ct.cumsumsaturate at 256 (the GPU returns 1024 for 1024 ones). - Tracing bf16/fp8 kernels:
tilelens.save()raises.- The visualizer silently receives empty tensors for bf16 Load/Store.
- A bf16 matmul makes
/api/datareturn 500.
ct.sum(mask)returnsTrueinstead of a count, andbool + intstays bool.- 0-d float64 results are narrowed to float32 (
ct.sum([1e300, ...])gives inf).
Silent divergences from GPU semantics
- Scalar typing:
- Python scalars are hard-coded to int32/float32, so int64 constants overflow and float64 tiles lose precision.
ct.bid()and kernel scalar arguments are treated as weak literals.
- Result dtypes and semantics of individual ops:
- bf16 matmul returns float32 instead of bf16.
- NaN handling in max/min/argmax differs.
- Float floordiv:
1.0 // 0.1gives 9 here and 10 on the GPU. bitwise_and/bitwise_orfollow different promotion rules frombitwise_xor.
- NumPy/torch dtype arguments raise
KeyError. This crashes 24/45 TileBench operators at commit 224ec81; all 45 pass on current TileBench main, which switched toct.*dtypes. - Code that should be rejected is accepted:
- Stores implicitly narrow (f32 into f16) where cuTile raises a compile error.
- Misspelled keyword arguments are silently swallowed.
- Coverage gaps:
- 43 public tile ops are unpatched and fail with a misleading error.
@ct.functionhelpers andfrom cuda.tile import ...do not work.
Biggest lever: run cuTile's own front-end as a pre-check, the way NKITrace does with pre_trace. It runs CPU-only and takes milliseconds. It rejects most of the "accepted here, rejected on GPU" cases, and it can give the interpreter the exact result types.
Tests, CI and packaging
- +1 to the request for end-to-end tests in
tests/end_to_end/. - Validate test kernels against real cuTile rather than only against the interpreter.
- Remove
pythonpath = .frompytest.ini. - Use a version range instead of the exact pin. Making cuTile a default dependency looks cheap; see the comment on
pyproject.toml. - Keep a CI job that runs without cuTile installed.
| result_tile = ct.full((TILE_M, TILE_N), 0, ct.float32) | ||
|
|
||
| # Accumulate partial sums. | ||
| for k in range(math.ceil(K / TILE_K)): |
There was a problem hiding this comment.
This kernel does not compile with real cuTile. I tested cuda-tile 1.5.0 via ct.launch on a 4090:
TileTypeError: Cannot create constant from value of type builtin_function_or_method.
examples/cutile/matmul.py, line 33, col 20-28, in matmul_kernel:
for k in range(math.ceil(K / TILE_K)):
The interpreter runs the kernel body as plain Python, so math.ceil works there. The README points users to this example and the tracer test imports it, so please use ct.cdiv(K, TILE_K) instead. With that change the kernel compiles and produces exact results on the GPU.
| @ct.kernel | ||
| def indexed_kernel(src, dst): | ||
| view = src.slice(0, 1, 4) | ||
| indices = ct.arange(4) - 1 |
There was a problem hiding this comment.
dtype is a required argument of ct.arange (_stub.py: def arange(size, /, *, dtype, start=0, step=1)). Real cuTile rejects this call with:
TileTypeError: arange(): missing a required keyword-only argument: 'dtype'
L96 has the same problem. It passes here only because Builder.arange gives dtype a default (see my comment there).
| def ones(self, shape, dtype): | ||
| return self.full(shape, 1, dtype) | ||
|
|
||
| def arange(self, size, *, dtype=ct.int32, start=0, step=1): |
There was a problem hiding this comment.
In cuTile, dtype is a required keyword-only argument. Defaulting it here lets kernels that will not compile on GPU pass under the interpreter; two of the kernels in this PR's own tests are examples. Please remove the default.
|
|
||
| def sum(self, x, /, axis=None, *, keepdims=False, **kwargs): | ||
| value = _tile_value(x) | ||
| return Tile(np.sum(value, axis=axis, keepdims=keepdims, dtype=value.dtype)) |
There was a problem hiding this comment.
Accumulating in value.dtype causes two problems:
-
bf16 saturates at 256. ml_dtypes' generic reduce adds the elements one at a time and rounds to bf16 after each step:
ct.sumof 1024 bf16 ones gives 256; the GPU gives 1024.ct.sumof 1024 × 0.1 gives 32.0; the GPU gives 102.5.
Any row sum of 512 or more elements (layernorm, softmax denominators) is off by 2–4x.
cumsum(L726) has the same problem. fp16 and fp32 are fine because they use NumPy's pairwise sum. -
bool.
ct.sum(mask)returnsTrueinstead of a count. The GPU promotes bool to int32.cumsumandmaxon bool tiles behave the same way.
Suggestion: accumulate bf16/fp8 in float32 and cast back afterwards, and follow cuTile's result-dtype rules for bool.
| value = np.asarray(value, dtype=_numpy_dtype(dtype)) | ||
| elif isinstance(value, int): | ||
| value = np.asarray(value, dtype=np.int32) | ||
| elif isinstance(value, float): |
There was a problem hiding this comment.
np.float64 is a subclass of Python float, so every 0-d float64 result that reaches this line is narrowed to float32. That includes ct.sum/ct.max over all axes, ops on 0-d tiles, and 1-D @ 1-D matmul:
ct.sum(float64 [1e300, 1, 1, 1]) -> GPU: float64 1e300 | interpreter: float32 inf
s + s with s = 1e-300 (0-d load) -> GPU: float64 2e-300 | interpreter: float32 0.0
test_mixed_float_tiles_do_not_narrow_float64 is meant to guard against this, but it only uses 1-element 1-D arrays. Checking type(value) is float instead of isinstance fixes it.
| kernel[grid](lhs, rhs, result) | ||
| ``` | ||
|
|
||
| Use contiguous NumPy arrays for visualization. cuTile analysis currently supports |
There was a problem hiding this comment.
A README note is easy to miss, and non-contiguous inputs do more than look off:
- Transposed or strided inputs highlight the wrong cells, because offsets are physical but get unraveled in C order. For example, block 0 of an
[::2]input highlights 0, 2, 4, 6 instead of 0–3. - Negative-stride inputs highlight almost nothing.
tilelens.saveraises.
A runtime warning, or rejecting non-contiguous arrays in the cuTile trace path, would be safer. Related: the visualizer derives element size from the dtype name, so cuTile bool_ arrays (and int16/fp8 arrays) are treated as 4-byte and highlight the wrong cells.
| @@ -0,0 +1,268 @@ | |||
| """Tests for cuTile tracing and visualization.""" | |||
There was a problem hiding this comment.
On test structure: +1 to Jokeren's request. Per the repo convention, kernel-level behavior should live in tests/end_to_end/ and unit tests in tests/unit/.
More importantly, every test here checks the interpreter only against itself or NumPy. That is how the example and two of these kernels ended up not compiling on real cuTile. Suggestions:
- Add end-to-end tests that trace real kernels, including bf16 kernels, and cover save/load plus the visualizer endpoints.
- Add a CPU-only check that each test kernel passes cuTile's front-end (
export_kernel), so the tests cannot drift from real cuTile.
Coverage also shows untested paths:
num_tiles,equal/__eq__,minimum/builtinmin,Tile.__matmul__- most reflected operators
- the unique-index
atomic_addpath tfloat32- the int-tile-with-float-scalar promotion branches
- most validation errors
| assert calls == [True] | ||
|
|
||
|
|
||
| def test_mixed_float_tiles_do_not_narrow_float64(): |
There was a problem hiding this comment.
This guards against float64 narrowing, but only with 1-element 1-D arrays. 0-d results are still narrowed (see my comment on Tile.__init__). Please add a case such as ct.sum over a float64 tile.
bd6e6f6 to
5c885bb
Compare
mark14wu
left a comment
There was a problem hiding this comment.
Thanks for the cuTile work, this is a big and useful addition. I went through the interpreter, the frontend adapter, and the tracer/visualizer changes. Inline comments below, roughly ordered by severity; most correctness items include a minimal repro.
Would like these addressed before merge
cuda-tileas a required dependency breaks installs on platforms without a wheel (pyproject.toml).- The shared
STORAGE_DTYPESadditions change NKI beta2 dtype validation (tilelens/utils/dtypes.py). interpreted_launchesaccumulates records across launches (tilelens/core/frontend/cutile.py).
Correctness
- Non-deterministic out-of-bounds fill with
PaddingMode.UNDETERMINED - Tuple/list array arguments are not wrapped as
Array - Unannotated int scalars are typed by magnitude (int64) instead of int32
atomic_cascompares numerically rather than bitwise; failed CAS lanes are traced as Stores- Kernel-scope patching reaches into third-party modules (e.g. torch)
- numpy-integer grid dims are rejected
flush_to_zero/RoundingMode.APPROXraiseNotImplementedError
Performance
get_offsets()is O(root array size) per traced access- Per-op
Signature.bindand per-Tilecopies
Cleanup
- Duplicated padding table / dtype helpers / root-array walk
tests/cutile/overlaps withtests/end_to_end/test_cutile.py
Minor: the PR checklist is still unchecked, could you update it?
| "flask_cloudflared", | ||
| "tqdm", | ||
| "ml_dtypes", | ||
| "cuda-tile>=1.0.0,<2", |
There was a problem hiding this comment.
[blocking] cuda-tile only publishes manylinux (x86_64/aarch64) and Windows wheels and has no sdist, so as a hard dependency pip install tilelens / uv sync --extra test fails on macOS and any other platform without a wheel, including for Triton-only users.
The code already treats cuTile as optional (HAS_CUTILE guards, pytest.importorskip), so I'd move this to [project.optional-dependencies] as a cutile extra. That would also make the pip install "tilelens[cutile]" command in docs/index.html work; right now that extra doesn't exist.
| "int8": np.dtype(np.int8), | ||
| "int16": np.dtype(np.int16), | ||
| "int32": np.dtype(np.int32), | ||
| "int64": np.dtype(np.int64), |
There was a problem hiding this comment.
[blocking] STORAGE_DTYPES is shared with the NKI beta2 simulator, where a missing key is exactly what makes _storage_dtype raise TypeError: Unsupported dtype. Adding "int64", "uint64" and "float8_e8m0fnu" here means e.g. nl.ndarray(shape, dtype="int64") now silently allocates instead of failing, so NKI kernels that wouldn't compile for Trainium pass in simulation.
Could the cuTile-only dtypes live in a cuTile-local table (e.g. STORAGE_DTYPES | {...} in the cuTile module) instead?
| runner = ( | ||
| kernel | ||
| if isinstance(kernel, CuTileTrace) | ||
| else tilelens.trace(client, frontend="cutile")(kernel) |
There was a problem hiding this comment.
[blocking] Each ct.launch builds a fresh CuTileTrace but reuses the same client, and the Tracer's records list is never reset between launches.
Repro: inside with interpreted_launches(Tracer()):, two ct.launch(None, (1,), k_copy, (src, dst)) calls give [len(l.records) for l in launches] == [3, 6]. The second Launch also contains the first launch's Grid/Load/Store records, so the visualizer shows duplicated accesses.
|
|
||
| # UNDETERMINED padding leaves out-of-bounds elements uninitialized. | ||
| if padding_mode == ct.PaddingMode.UNDETERMINED: | ||
| value = np.empty(access_shape, dtype=data.dtype) |
There was a problem hiding this comment.
With the default PaddingMode.UNDETERMINED, the out-of-bounds lanes of a partial tile come from np.empty, i.e. arbitrary memory. Example: loading a (4,) tile at index 1 from a 6-element array returned [4.0, 5.0, 1.4e-45, 0.0]. A partial-tile ct.sum / ct.max / ct.matmul then gives run-to-run different (possibly NaN) results, and recorded Load/Dot data isn't reproducible.
Since the value is undefined anyway, a deterministic fill (e.g. NaN for floats, a sentinel for ints) would keep traces stable and make reliance on undefined lanes visible. Alternatively raise, as load_advanced_indexing already does for this case.
| return Array(arg.view(storage[arg.dtype]).numpy().view(dtype)) | ||
| return Array(arg.numpy()) | ||
| # Like Triton's _implicit_cvt, leave other values (tuples, dtypes) unchanged. | ||
| return arg |
There was a problem hiding this comment.
Tuples and lists are passed through unchanged, so numpy arrays inside them are never wrapped as Array (and never registered by the tracer).
Repro: def k(arrs, dst): ct.load(arrs[0], (0,), (4,)) with CuTileInterpretedFunction(k).run((np.arange(4.),), dst) raises AttributeError: 'memoryview' object has no attribute 'transpose' (it ends up on ndarray.data). cuTile supports tuple/list-of-array parameters, so this probably needs to recurse into tuples/lists.
| raise TypeError(f"Invalid {key}: {bound.arguments[key]}") | ||
| cutile_builder._check_cpu_math_options( | ||
| bound.arguments.get("rounding_mode"), | ||
| bound.arguments.get("flush_to_zero", False), |
There was a problem hiding this comment.
flush_to_zero=True and rounding modes other than RN/FULL (e.g. APPROX) raise NotImplementedError, so attention/softmax-style kernels that use these flags can't be interpreted or traced at all. On CPU, APPROX could fall back to the exact result, and FTZ could be emulated by flushing subnormals (or ignored with a one-time warning). Not blocking, but it would widen coverage a lot.
| return self.data.astype(np.float32) | ||
| return self.data | ||
|
|
||
| def get_offsets(self): |
There was a problem hiding this comment.
get_offsets() materializes byte offsets for the entire root array, and the tracer calls it on every traced Load/Store, so tracing cost is O(blocks x accesses x array size) rather than O(tile size).
Measured: a 1024x1024 tiled copy takes 0.09s untraced vs 1.26s traced. Extrapolating, 4096x4096 with 32x32 tiles is ~32k calls at ~30ms each, about 16 minutes. Computing offsets only for the accessed tile (sum(index * stride) over the access) would avoid this.
|
|
||
| @wraps(original) | ||
| def execute(*args, **kwargs): | ||
| bound = _LANG_SIGNATURES[name].bind(*args, **kwargs) |
There was a problem hiding this comment.
Minor perf: every ct op runs Signature.bind + apply_defaults just to validate a few kwargs, and Tile.__init__ (L281) always copies its input, including freshly computed temporaries like Tile(operation(x, y).astype(dtype)). In elementwise-heavy loops that's a few microseconds per op plus an extra allocation and copy per intermediate. Validating only the enum/rounding kwargs when they're present, and a no-copy path for internally constructed Tiles, would remove most of it.
| ) | ||
| padding = 0 | ||
| else: | ||
| modes = { |
There was a problem hiding this comment.
Nit: this padding-mode-to-value table and the "nonzero padding requires a floating-point array" check duplicate Builder.load (~L684). Similarly, _is_float and _is_low_precision repeat the same four dtypes, and the while root._parent walk appears in both adapters in frontend/cutile.py (L44, L106), mirroring the tracer's _base_tensor. A module-level _PADDING_VALUES and a shared _root(array) helper would keep the two load paths from drifting apart.
| check_visualization() | ||
|
|
||
|
|
||
| def copy_kernel(src, dst): |
There was a problem hiding this comment.
AGENTS.md puts kernel-level behavior in tests/end_to_end/. This new tests/cutile/ directory overlaps with tests/end_to_end/test_cutile.py (both define copy/matmul kernels and both check the visualizer). Could these tests move into tests/end_to_end/test_cutile.py, dropping the third path from the README?
This change does not include TileBench
Summary
Test Plan
tests/cutile/test_cutile.py.tests/cutile/test_cutile_tracer.py, covering kernel execution, trace save/load, and visualizer APIs.Related Issues
None.
Breaking Changes
None.
Checklist
npm run build:frontendif the PR modified any TypeScript code.