flydsl qr int4 - #2
Conversation
🏷️ CI GuideRuns automatically on every PR:
Extended tests (opt-in via labels):
PR title tags: |
Drop unused wave-join and single-wave fanout compiles; default and test the fp16 codec.
Compile-only and cache warming belong in developer scripts, not pytest or QRInt4QuadFanout.compile. Co-authored-by: Cursor <cursoragent@cursor.com>
inttoptr + make_buffer_tensor (nbytes OOB) and BufferCopy128b tiled copies replace the tile/atom/tid arithmetic for local dwordx4 loads and stores. IPC/XGMI fanout stays global_store_dwordx4 nt.
4ac7f72 to
823c143
Compare
…hey were not FlyDSL-main originals.
…peline so the public API is QRInt4.
… measured keeper; drop unused codec/force_super/bf16 ALU and qr_int4_mem, compiling via flyc.compile. Co-authored-by: Cursor <cursoragent@cursor.com>
…R does the same), and time QRInt4 with run_perftest.
There was a problem hiding this comment.
Pull request overview
Adds a gfx942 TP8 FlyDSL INT4 QuickReduce all-reduce implementation.
Changes:
- Implements the INT4/E4M3 two-shot GPU kernel and host launcher.
- Adds uncached HIP IPC buffer management.
- Exports
QRInt4and adds correctness/benchmark coverage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
op_tests/flydsl_tests/test_flydsl_qr_int4.py |
Adds TP8 correctness and benchmark tests. |
aiter/ops/flydsl/kernels/qr_int4.py |
Adds the public host-side engine. |
aiter/ops/flydsl/kernels/qr_int4_kernel.py |
Implements the FlyDSL reduction kernel. |
aiter/ops/flydsl/kernels/qr_int4_ipc.py |
Implements HIP IPC allocation and exchange. |
aiter/ops/flydsl/__init__.py |
Exports QRInt4. |
Suppressed comments (4)
aiter/ops/flydsl/kernels/qr_int4.py:177
allreduceforwards rawdata_ptr()values but only validates dtype and byte count. A CPU bf16 tensor or a non-dense view such asbase[:, ::2]passes these checks, so the GPU kernel receives an invalid pointer or reduces the wrong physical bytes. Require CUDA tensors on the engine's device and dense (or explicitly supported weak-contiguous) input/output layouts before launch.
def allreduce(self, inp: torch.Tensor, out: torch.Tensor, stream=None):
if inp.dtype != torch.bfloat16 or out.dtype != torch.bfloat16:
raise ValueError("QRInt4 supports bf16 input/output")
live_bytes = int(inp.numel()) * int(inp.element_size())
if live_bytes % 16 != 0:
raise ValueError("byte size must be a multiple of 16 (8 bf16)")
if int(out.numel()) * int(out.element_size()) != live_bytes:
raise ValueError("inp/out byte size mismatch")
aiter/ops/flydsl/kernels/qr_int4.py:139
- Every launch reuses the engine's color counters and the same IPC inbox. Two host calls submitted to different CUDA streams are unordered, so both kernels can observe the same color and overwrite the same phase slots, causing incorrect output or a collective hang. Serialize launches with an event/stream wait, or explicitly reject changing streams for an engine.
if stream is None:
stream = torch.cuda.current_stream()
op_tests/flydsl_tests/test_flydsl_qr_int4.py:258
- The payload contains results from all eight ranks, but this return discards ranks 1–7, so every SQNR and super-tile assertion only checks rank 0. A rank-dependent peer-map or gather defect can therefore pass the new correctness test. Return or aggregate every rank's rows and assert each rank.
return payload["ranks"][0]
op_tests/flydsl_tests/test_flydsl_qr_int4.py:207
- Removing
HIP_VISIBLE_DEVICESdiscards the GPU set assigned by a scheduler. For example, a job assigned physical GPUs 8–15 will make each child use physical GPUs 0–7 instead, potentially colliding with another job. Preserve the inherited visibility mapping;cuda:{rank}already indexes that mapped set.
env.pop("HIP_VISIBLE_DEVICES", None)
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| try: | ||
| gloo = dist.new_group(backend="gloo") | ||
| except Exception: # noqa: BLE001 | ||
| gloo = dist.group.WORLD |
| if world_size != WORLD: | ||
| raise ValueError( | ||
| f"only world_size={WORLD} is implemented, got {world_size}" | ||
| ) | ||
| if super_tile not in SUPER_TILES: | ||
| raise ValueError( | ||
| f"super_tile must be one of {SUPER_TILES}, got {super_tile!r}" | ||
| ) |
| rank = dist.get_rank(group=group) | ||
| all_data = [[None] for _ in range(world_size)] | ||
| all_data[rank][0] = shard_data | ||
| ranks = sorted(dist.get_process_group_ranks(group=group)) |
…; WORLD=8 stays kRankAtoms=1. Co-authored-by: Cursor <cursoragent@cursor.com>
| if b is not None: | ||
| try: | ||
| UncachedIpcHeap.close_mem_handle(int(b)) | ||
| except RuntimeError: # noqa: S110 |
| if self._buf_ptr: | ||
| try: | ||
| UncachedIpcHeap.free_device_mem(self._buf_ptr) | ||
| except RuntimeError: # noqa: S110 |
…aiter vendored dual header. Co-authored-by: Cursor <cursoragent@cursor.com>
gfx942 already lowers vector<2xf16> to v_pk_max/min/mul/add/fma, so the llvm.inline_asm helpers and f32 absmax round-trip are unnecessary.
No description provided.