Skip to content

[Bugfix] Show the user's source line in DSL compile errors - #1337

Open
RElbers wants to merge 1 commit into
mainfrom
location-chain-typed-bindings
Open

RElbers wants to merge 1 commit into
mainfrom
location-chain-typed-bindings

Conversation

@RElbers

@RElbers RElbers commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

DSLCompileError now points at the user's source line, for verifier errors and for errors from the pass pipeline. Generated code is unchanged: this only affects error messages, plus source locations in IR dumps. On main it never does. For a kernel that builds an invalid arith.addi, main prints only the MLIR message:

'arith.addi' op requires the same type for all operands and results

With this PR it also prints the MLIR note and DSL traceback:

'arith.addi' op requires the same type for all operands and results
  note: see current operation: %2 = "arith.addi"(%0, %1) <{overflowFlags = #arith.overflow<none>}> : (i32, i64) -> i32

DSL Traceback (most recent operation last):
  File "kernel.py", line 14
    ir.Operation.create("arith.addi", results=[a.type], operands=[a, b])
    ^

Ops built by fx.* primitives always carry the line that called them. A raw ir.Operation.create, as here, gets its own line only with FLYDSL_DEBUG_ENABLE_DEBUG_INFO=1. Otherwise the traceback points at the @flyc.kernel line.

Motivation

Three things lose the location:

  • location_chain uses Location.is_a_callsite() and similar methods, which the MLIR Python bindings removed in favor of typed subclasses (CallSiteLoc, ...). A catch-all except hid the error, so every location resolved to no frames. Verifier errors also hold plain ir.Locations, which match no subclass.
  • PassManager.run collects pass-pipeline errors into its MLIRError before FlyDSL's diagnostic handler sees them, so those errors produce no records.
  • The module is re-parsed without locations before the pipeline, so every op the passes see is at loc("-":N:M).

Changes

  • location_chain rebuilds each location as its typed subclass (Location.from_attr) and no longer swallows exceptions.
  • Pass-pipeline errors are built from the MLIRError's diagnostics, in one helper used by both the pipeline and the FLYDSL_DUMP_IR stage loop.
  • MLIR's notes, such as "see current operation", are kept on one line; a note that prints an op with regions (a whole gpu.module, say) leaves out the body.
  • The module is re-parsed with its locations. They only become DWARF when the debug-info pass runs, and the JIT cache and AOT exports don't store them.

Out of scope

With FLYDSL_COMPILE_LLVM_DIR (external codegen), the verify() after codegen still raises a bare MLIRError. It checks the module after the gpu.module has been replaced by its binary, so a failure there comes from FlyDSL or the external tool, not from user code, and there's no user source line to point at.

Performance

Compile time grows slightly, because the passes now carry source locations. Timing MlirCompiler.compile for each of the 273 kernels of tests/kernels/test_flash_attn_fwd.py on MI355X, best of two runs on each side, the median compile goes from 260.9 to 268.9 ms; the median paired difference is 7.2 ms (2.5%). The cost scales with the amount of IR, so a small kernel shows no measurable difference. Generated code is unchanged.

Testing

  • Unit tests added/updated:

    • tests/unit/test_diagnostics.py: call-site, name and fused locations resolve to frames, synthetic and unknown ones to none; a real verifier failure keeps its source chain.
    • tests/unit/test_compile_error_locations.py: a pass-pipeline failure keeps its source chain; a kernel that fails LLVM translation raises a DSLCompileError pointing at its file; without debug info, the IR the first pass sees still has user locations.

    On main, every new test fails except the two that check skipped locations.

  • Full scripts/run_tests.sh set on MI355X.

  • Compile time measured as above

  • Tested on MI355X and cross-compiled for gfx942

Dependencies

  • No new third-party dependencies added

Breaking Changes

None.

@RElbers
RElbers force-pushed the location-chain-typed-bindings branch 2 times, most recently from 3964207 to e38779f Compare October 27, 2026 11:33
- Rebuilds each location as its typed subclass and checks it with isinstance, since the bindings no longer have Location.is_a_callsite() and friends, and the locations an MLIRError carries come back as plain Location.
- Drops the catch-all exception handlers in location_chain and diag_record_from_diagnostic, which hid the missing methods by resolving every location to no frames.
- Builds pass-pipeline error records from the MLIRError's own diagnostics, since PassManager.run captures errors before any context handler sees them, in one helper shared by the pipeline and the IR-dump stage loop.
- Keeps each error's MLIR notes, such as "see current operation", on one line, leaving out the body of any region they print.
- Re-parses the module with its source locations before the pass pipeline, so pass diagnostics point at user code; locations only become DWARF when the debug-info pass runs.
- Covers typed, synthetic and unknown locations, a real verifier failure, a real pass-pipeline failure, a kernel that fails LLVM translation, and the locations the first pass sees without debug info.

Without this, no DSLCompileError showed a source line: verifier errors lost their locations in the bindings, pass errors never reached the records, and the pipeline ran on IR whose locations pointed into its own printed text.
@RElbers
RElbers force-pushed the location-chain-typed-bindings branch from e38779f to 8e3df81 Compare October 27, 2026 12:25
@RElbers
RElbers marked this pull request as ready for review October 27, 2026 12:28
@RElbers
RElbers requested a review from sjfeng1999 October 27, 2026 12:33
@coderfeli coderfeli added the ai-review Request an advisory AI review (review-pr workflow); never a merge gate label Oct 27, 2026
@coderfeli
coderfeli deployed to flydsl-review October 27, 2026 15:08 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown
Contributor

FlyDSL PR #1337 — [Bugfix] Show the user's source line in DSL compile errors

Makes DSLCompileError print the MLIR note and a DSL traceback pointing at the user's source line, for verifier failures and pass-pipeline failures, by fixing location decoding, capturing pass errors from MLIRError, and keeping source locations when the module is re-parsed.

Review (advisory): ✅ NO FINDINGS above note level (two 📝 notes below)
CI (head 8e3df81): mi35x-1 test SUCCESS; navi-2 test FAILURE (not classified: its log was not readable, so PR vs baseline vs infra is unknown); mi325-1 test IN_PROGRESS; style, docs and CodeQL SUCCESS. The GPU matrix does not run on gfx1250, and the multi-gpu and downstream integration jobs do not run on PRs.
Perf (CI benchmark, advisory): NOT MEASURED — no kernels/ file changed, so no benchmark reaches this change; kernel code is unaffected (a local compile of the PR's kernel gave identical LLVM IR and ISA with and without the re-parse locations), and the compile-time cost is the 📝 below.

📝 [inferred] The "7.4 ms (2%) median per kernel over 291 kernels on MI355X" compile-time figure in the PR body cannot be traced to a log or script in the PR, and python/flydsl/compiler/jit_function.py:830 now makes every compile print and re-parse the module with locations; my five runs on a one-op kernel showed no measurable difference, which says nothing about large kernels. Reviewer should ask the author to attach the timing script and output, or mark the number as approximate.
📝 [verified] The post-codegen module.operation.verify() calls at python/flydsl/compiler/jit_function.py:906 and :958 (the FLYDSL_COMPILE_LLVM_DIR external-codegen paths) still raise a bare MLIRError rather than a DSLCompileError, so a failure there prints no DSL traceback even though the module now carries user locations; MlirCompiler.compile at line 820 and the pass runner were converted. This is pre-existing and limited to the external-codegen mode. Author must either route those two verifies through the same diag_records_from_mlir_error wrapping or state that external codegen is out of scope.

Independent reader's corrections (not yet applied to the findings above):

  • Finding 1: the print-and-re-parse step already ran on every compile in merge-target (get_asm(enable_debug_info=env.debug.enable_debug_info) then Module.parse); the PR only changes it to always emit locations, so say "now always prints and re-parses with locations" rather than "now makes every compile print and re-parse".

Advisory review by the review-pr skill with Claude-Sonnet-5.5, requested by @coderfeli with the ai-review label. Not a merge gate. Run

This branch was successfully deployed

1 active deployment
flydsl-review — 8e3df813 Deployed Oct 27, 2026 by coderfeli via review #9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request an advisory AI review (review-pr workflow); never a merge gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants