Skip to content

Clear lowering-pass side tables between functions (intermittent 00try_finally_return failure) - #317

Merged
ASDAlexander77 merged 1 commit into
mainfrom
fix-stale-lowering-context
Sep 13, 2026
Merged

ASDAlexander77 merged 1 commit into
mainfrom
fix-stale-lowering-context

Conversation

@ASDAlexander77

Copy link
Copy Markdown
Owner

Problem

00try_finally_return.ts failed now and then on Windows CI, only in AOT compile variants, and passed on re-run (first attempts of the #310 and #312 runs):

00try_finally_return.ts:23:5: error: reference to block defined in another region
    try {

It never failed locally.

Cause

The affine lowering keeps side tables in TSContext that decide where ops branch: unwind, cleanup, jumps, parentTryOp, landingBlockOf, leavesCatch, catchOpData. They are keyed by Operation* and hold Block* values. They were never cleared, and TSContext is a member of the pass object, which lowers every function on its thread in turn.

Once func1 is lowered its ops are freed, but its entries stay. An op created while lowering func2 can reuse the address of one of those ops and inherit its entry, so it branches into one of func1's blocks, which is the verifier error above.

Reproduced single-threaded (--mlir-disable-threading) with the old behaviour: compiling all 509 test files produced exactly one stale lookup, on two different builds, always the same site:

00try_finally_return.ts: stale lowering-context entry: cleanup for 'ts.Return' at 00try_finally_return.ts:23:5

That ts.Return is the one TryOpLowering creates at the end of func2's return-path finally copy (hence the try's location). It inherited the cleanup entry of func1's return, which pointed at func1's finally copy.

Why only CI: MLIR gives each worker thread its own pass clone. On a 16-thread machine the functions of a small file rarely share a clone; CI runners have few cores. Even single-threaded the hit depends on heap layout: the same binary hit on 2 of 100 compiles of this file.

Fix

  • Every TSContext table is now an OpSideTable<V>. TSContext::beginRun() clears all of them, and the three affine lowering passes call it before building their patterns, i.e. once per function (or once per module).
  • Each entry records the run that wrote it. A lookup/contains that still finds an entry from an earlier run prints the table, op and location and stops with report_fatal_error, in every build: in release this used to be a silent miscompile at best.
  • Reads use lookup/contains instead of operator[], which inserted an empty entry on every read. Writes still use [].
  • TSLANG_REPORT_STALE_LOWERING_CONTEXT prints how many entries each run drops from the one before.

Tests

  • Full release suite (Windows): 2716/2716 passed, no stale reports.
  • All 509 test files compiled single-threaded: 0 stale lookups with the fix (1 without).
  • No regression test: whether a freed address is reused is random, and a single-threaded ctest entry still passed with the fix reverted, so it was dropped. The fatal check is the guard; the real confirmation is 00try_finally_return no longer failing on CI.

🤖 Generated with Claude Code

@ASDAlexander77
ASDAlexander77 merged commit 99855a5 into main Sep 13, 2026
2 checks passed
@ASDAlexander77
ASDAlexander77 deleted the fix-stale-lowering-context branch September 13, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant