Skip to content

Commit 8eec98b

Browse files
authored
[DOC] Clean up MLA internal type usage and docs (#460)
1 parent f7db21e commit 8eec98b

14 files changed

Lines changed: 369 additions & 269 deletions

File tree

.claude/skills/debug-flydsl-kernel/SKILL.md

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ name: debug-flydsl-kernel
33
description: >
44
Debug FlyDSL GPU kernels that produce NaN, inf, wrong results, or crash.
55
Covers cache invalidation, tracing pitfalls (runtime conditionals, range vs
6-
range_constexpr), scf.for state packing, buffer_load addressing, MFMA operand
6+
range_constexpr), loop-carried state packing, buffer_load addressing, MFMA operand
77
layout verification, LDS bank conflict diagnosis, and systematic error
88
isolation (all-1s test, single-partition test, host-side tensor inspection).
99
Use when a FlyDSL kernel produces incorrect output or compilation errors.
@@ -34,7 +34,7 @@ compile_my_kernel.cache_clear()
3434
| All zeros output | Wrong output address, uninitialized temp buffer | Section 3 |
3535
| Partially wrong (>50% mismatch) | Wrong partition count, missing partitions, layout mismatch | Section 4 |
3636
| Small errors (1-5% mismatch) | FP8 quantization, scale factor, off-by-one masking | Section 5 |
37-
| Compilation error / crash | Type mismatch, scf.for state, range vs range_constexpr | Section 6 |
37+
| Compilation error / crash | Type mismatch, loop-carried state, range vs range_constexpr | Section 6 |
3838
| GPU hang | Infinite loop, deadlock in barrier, OOB memory access | Section 7 |
3939

4040
## 2. Debugging NaN
@@ -45,7 +45,7 @@ When ALL tokens in a partition are masked (out of context), `qk_max = -inf`. The
4545

4646
**Fix**: Guard the exp calculation:
4747
```python
48-
safe_diff = arith.select(qk_max > NEG_INF, diff, ZERO_F)
48+
safe_diff = (qk_max > NEG_INF).select(diff, ZERO_F)
4949
```
5050

5151
### 2.2 Division by zero in normalization
@@ -54,8 +54,8 @@ When `exp_sum = 0` (all probs zero), `1/exp_sum = inf`.
5454

5555
**Fix**:
5656
```python
57-
safe_sum = arith.select(running_sum > ZERO_F, running_sum, arith.constant(1.0, type=T.f32))
58-
inv_sum = arith.constant(1.0, type=T.f32) / safe_sum
57+
safe_sum = (running_sum > ZERO_F).select(running_sum, fx.Float32(1.0))
58+
inv_sum = fx.Float32(1.0) / safe_sum
5959
```
6060

6161
### 2.3 Host-side NaN check
@@ -143,7 +143,7 @@ Verify `_scale = softmax_scale * q_scale * k_scale` matches the reference. Commo
143143

144144
### 6.1 `range()` vs `range_constexpr()` inside @flyc.kernel
145145

146-
FlyDSL's AST rewriter converts ALL `range()` to `scf.for` (runtime loops). Use `range_constexpr()` for compile-time unrolled loops:
146+
FlyDSL's AST rewriter converts runtime `range()` loops into MLIR loops. Use `range_constexpr()` for compile-time unrolled loops:
147147
```python
148148
# WRONG: i becomes an ArithValue, can't index Python lists
149149
for i in range(4): result[i] = ...
@@ -160,23 +160,23 @@ FlyDSL tracing evaluates Python `if` at trace time. Runtime GPU values can't be
160160
if kv_tok < context_len: # runtime comparison
161161
fx.printf(...)
162162

163-
# CORRECT: use arith.select for runtime conditionals
164-
val = arith.select(kv_tok < context_len, good_val, bad_val)
163+
# CORRECT: use ArithValue.select for runtime value selection
164+
val = (kv_tok < context_len).select(good_val, bad_val)
165165
```
166166

167167
Python `if` is fine for COMPILE-TIME decisions (e.g., `if trans_v:` where trans_v is a Python bool).
168168

169-
### 6.3 scf.for state packing
169+
### 6.3 Loop-carried state packing
170170

171-
All loop-carried values must be raw SSA values (not Python wrappers):
171+
Prefer FlyDSL internal types (`fx.Int32`, `fx.Float32`, `Vector`, `ArithValue`) for loop-carried state. Unwrap only when a low-level helper explicitly requires raw `ir.Value`:
172172
```python
173173
def _unwrap(v):
174174
return v.ir_value() if hasattr(v, 'ir_value') else v
175175

176176
init_state = [_unwrap(v) for v in [val1, val2, vec_val]]
177177
```
178178

179-
Supported state types: `f32` (scalar), `f32x4` (vector), `i32`, `i64`, `index`.
179+
Supported state types: `f32` (scalar), vector values, `i32`, `i64`, `index`.
180180

181181
### 6.4 buffer_load type mismatch
182182

@@ -186,21 +186,21 @@ k_addr_bytes = ... # address in FP8 elements (= bytes for FP8)
186186
k_4xi32 = buffer_ops.buffer_load(k_rsrc, k_addr_bytes // 4, vec_width=4, dtype=T.i32)
187187
```
188188

189-
### 6.5 vector.store requires vector type
189+
### 6.5 Vector stores require vector values
190190

191-
LDS `vector.store` requires the value to be a vector, not scalar:
191+
`Vector.store` requires the value to be a vector, not scalar:
192192
```python
193193
# WRONG
194-
vector.store(scalar_i32, lds_ptr, [idx])
194+
Vec(scalar_i32).store(lds_ptr, [idx])
195195

196196
# CORRECT
197-
vec = vector.from_elements(T.vec(1, T.i32), [scalar_i32])
198-
vector.store(vec, lds_ptr, [idx])
197+
vec = Vec.from_elements([scalar_i32], fx.Int32)
198+
vec.store(lds_ptr, [idx])
199199
```
200200

201201
## 7. GPU Hang
202202

203-
### 7.1 Infinite scf.for loop
203+
### 7.1 Infinite runtime loop
204204

205205
If loop bounds are wrong (`stop < start` with unsigned comparison issues, or `step=0`), the GPU hangs. Verify bounds on host:
206206
```python
@@ -239,8 +239,8 @@ sudo amdgpu-reset # or reboot
239239
- [ ] `range_constexpr()` for all compile-time loops (not `range()`)
240240
- [ ] No Python `if` on runtime GPU values
241241
- [ ] `buffer_load` offset units match dtype (bytes/4 for i32)
242-
- [ ] `vector.store` uses vector type (not scalar)
243-
- [ ] `scf.for` state packed with `_unwrap()` (raw SSA values)
242+
- [ ] Vector stores use `Vector` values (not scalars)
243+
- [ ] `range(..., init=...)` state uses internal types, unwrapped only at hard boundaries
244244
- [ ] Output written to correct partition slot (`part_z`, not absolute index)
245245
- [ ] `exp_sums`/`max_logits` strides match actual tensor layout
246246
- [ ] Softmax guards against `-inf - (-inf) = NaN`
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
---
2+
name: flydsl-internal-types-cleanup
3+
description: >
4+
Clean up FlyDSL kernel code by replacing direct scf/arith/vector/memref dialect calls
5+
with FlyDSL internal types and helpers while preserving correctness, performance, and
6+
generated ASM. Use when refactoring FlyDSL kernels, removing redundant wrappers, or
7+
updating docs/skills to prefer fx.Int32/fx.Index/fx.Float32, ArithValue, Vector, and
8+
range(..., init=...).
9+
---
10+
11+
# FlyDSL Internal Types Cleanup
12+
13+
## Default Rule
14+
15+
Prefer FlyDSL internal types and high-level helpers in kernel code:
16+
17+
- Constants and casts: `fx.Int32`, `fx.Int64`, `fx.Index`, `fx.Float32`
18+
- Arithmetic and comparisons: Python operators on `ArithValue` / `Numeric`
19+
- Runtime select: `cond.select(a, b)` or `arith.select(...)` only when a helper boundary requires it
20+
- Vectors: `Vector` (`Vec`) indexing, `Vec.from_elements`, `Vec.filled`, `.bitcast(...)`, `.to(...)`, `.store(...)`
21+
- Register memory: `fx.memref_alloca`, `fx.memref_load_vec`, `fx.memref_store_vec`
22+
- Runtime loops with carried state: `range(start, stop, step, init=[...])` using `fx.Index(...)` bounds
23+
- Compile-time loops: `range_constexpr(...)`
24+
25+
Avoid new direct `scf.*`, `vector.*`, `memref.*`, `arith.index`, `arith.index_cast`, and `arith.trunc_f` in kernel bodies unless a lower-level boundary requires the exact op.
26+
27+
## Replacement Map
28+
29+
| Low-level form | Preferred form |
30+
|---|---|
31+
| `arith.constant(0, type=T.i32)` | `fx.Int32(0)` |
32+
| `arith.constant(0, index=True)` / `arith.index(0)` | `fx.Index(0)` |
33+
| `arith.constant(1.0, type=T.f32)` | `fx.Float32(1.0)` |
34+
| `arith.index_cast(T.i32, x)` | `fx.Int32(x)` |
35+
| `arith.index_cast(T.index, x)` | `fx.Index(x)` |
36+
| `vector.extract(v, static_position=[i], ...)` | `Vec(v)[i]` |
37+
| `vector.bitcast(T.vec(...), v)` | `Vec(v).bitcast(fx.Int32)` etc. |
38+
| `vector.from_elements(T.vec(n, T.i32), xs)` | `Vec.from_elements(xs, fx.Int32)` |
39+
| `vector.store(v, memref, [idx])` | `Vec(v).store(memref, [idx])` |
40+
| `arith.trunc_f(T.bf16x4, v)` | `Vec(v).to(fx.BFloat16)` |
41+
| `arith.addf/mulf` | `a + b`, `a * b` |
42+
| `arith.select(cond, a, b)` | `cond.select(a, b)` when `cond` is an `ArithValue` |
43+
44+
## Important Exceptions
45+
46+
Keep the exact lower-level op when it encodes semantics that internal types do not expose:
47+
48+
- `llvm.InlineAsmOp` for hand-scheduled ISA snippets
49+
- `llvm.LoadOp` / `llvm.StoreOp` when `volatile`, `nontemporal`, address space, or alignment must be explicit
50+
- `arith.*FOp(..., fastmath=...)` when performance depends on fastmath flags
51+
- `arith.DivUIOp` / `arith.RemUIOp` for unsigned integer division/remainder
52+
- `rocdl.*` intrinsics and MFMA/WMMA/TDM ops
53+
- Backend dialect/C++ lowering docs and implementation code
54+
55+
Do not hide these exceptions behind new helper wrappers just to remove the visible op. If exact semantics are required, keep the direct op at the boundary and document why.
56+
57+
## Control Flow
58+
59+
- Compile-time / constant conditions must be written as `if const_expr(condition): ...`. Do not rely on a plain Python `if` unless the condition is already a Python `bool`.
60+
- Use ordinary Python `if` on runtime values only when the AST rewriter keeps branch-local values and side effects correct.
61+
- For runtime branches inside nested helper functions, wrap the dispatch in a local `@flyc.jit` helper. This keeps branch side effects and loop-carried state in the right rewritten region.
62+
- For complex runtime branches with side effects, loop-carried state, or branch-local definitions, split branch bodies into local helper functions and dispatch through a local `@flyc.jit` helper. Verify correctness and ASM/perf.
63+
- Do not hand-write `scf.IfOp` in new kernel code unless the `@flyc.jit` helper pattern cannot express the required branch.
64+
- Use `range(..., init=[...])` for runtime loops with carried state; unwrap init values only if the API specifically requires raw `ir.Value`.
65+
66+
Pattern:
67+
68+
```python
69+
def _then_path():
70+
...
71+
72+
def _else_path():
73+
...
74+
75+
@flyc.jit
76+
def _dispatch():
77+
if runtime_cond:
78+
_then_path()
79+
else:
80+
_else_path()
81+
82+
_dispatch()
83+
```
84+
85+
## Verification Loop
86+
87+
For performance-sensitive kernels:
88+
89+
1. Record baseline shape coverage, timing, ASM hash, VGPR/SGPR counts, and spill counts.
90+
2. Apply one cleanup group at a time.
91+
3. Run correctness on small and large representative shapes.
92+
4. Compare performance; for strict cleanups, compare ASM hash.
93+
5. If performance drops or results change, revert that cleanup group and keep the lower-level op.
94+
95+
Recommended checks:
96+
97+
```bash
98+
PYTHONPATH=python:. FLYDSL_RUNTIME_ENABLE_CACHE=0 <kernel test command>
99+
FLYDSL_DUMP_IR=1 FLYDSL_RUNTIME_ENABLE_CACHE=0 PYTHONPATH=python:. <small compile command>
100+
sha256sum ~/.flydsl/debug/<kernel>/21_final_isa.s
101+
```

0 commit comments

Comments
 (0)