Skip to content

fix(cutile-compiler): preserve origins of short-circuit, folded and bool-binding constants - #1528

Merged
elibol merged 3 commits into
NVIDIA:mainfrom
GiGiKoneti:fix/cutile-short-circuit-spans
Oct 29, 2026
Merged

elibol merged 3 commits into
NVIDIA:mainfrom
GiGiKoneti:fix/cutile-short-circuit-spans

Conversation

@GiGiKoneti

@GiGiKoneti GiGiKoneti commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Short-circuit and folded constants can inherit a synthetic type's module-header location or an operand's definition instead of the expression that produces them. Pass the expression span into constant construction so Line and Full debug modes preserve the expression's file, line and nested inline callers.

Const-bool bindings in entries, functions and methods use the corresponding callee signature. Resolving caller coordinates in a callee scope could otherwise attach the binding to the wrong file/line.

The new constant_source_locations CPU CI target checks deciding false/true constants, a folded expression on a different line from its operands, and all three binding contexts. It verifies exact cross-file caller chains both in the IR and after bytecode serialization.

Validation: all three regressions pass and fail on the earlier compiler; selectively restoring the old folded-expression span fails its dedicated regression. Existing debug/generated-source tests, assembler/GPU/CUDA-GDB checks, formatting and strict Clippy pass on CUDA 13.3. Combined debug validation in #1613 passes all 35 tests. CI and review remain required before merge.

Part of #1475; v0.5.0 tracking: #1606.

@copy-pr-bot

copy-pr-bot Bot commented Oct 16, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@elibol

elibol commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Reviewing this against the short-circuit item assigned in #1475. The new test checks a direct || constant and an inlined && constant, including the caller line, which matches that scope.

Before considering those sub-items covered:

  • Run the exact-location checks in both Line and Full, and add a helper from another file. Assert the file, subprogram, and complete nested caller chain as well as the expression line, then verify the relevant location survives bytecode serialization. Include both deciding values (false for &&, true for ||).
  • The diff also changes the constant-folding call to compile_constant_from_exact_bounds to pass the expression span. Please add a dedicated regression that forces that path (for example, a folded arithmetic expression used by the kernel), checks its original expression location and inline chain, and fails before this change. The current boolean test does not establish the constant-folding item in cuTile: audit generated source locations (optional follow-ups to #1379) #1475. If that change remains, describe both covered items in the PR.
  • The shared-builder signature also updates const-bool entry/function/method bindings. Exercise those paths to pin the intended binding/call-site locations, and check compatibility with fix: correct cuTile source locations for generated constants #1385's location fallback rather than relying on it to supply the expression span.

Please record the focused fail-before/pass-after result. Wrapper locations, arrays, reductions, and injected checks remain separate #1475 work; this PR should not close the entire audit or claim to fix backend assertion traps.

@elibol

elibol commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 64f0ebf

@elibol

elibol commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Pushed 64f0ebf4ae61541eb2b2fe9c2ae4cac465a09e12, merging current main and addressing the outstanding test feedback.

Fixed a const-bool attribution issue in the proposed change: function/method bindings resolved a caller expression span inside the callee scope. They now use the callee signature, matching const-array bindings.

The new CPU CI target checks deciding false/true short-circuit constants, a folded expression on a different line from both operands, and entry/function/method const-bool bindings. It covers Line/Full, cross-file nested callers and exact serialized chains. Regressions fail on the earlier compiler; reverting only the folded-expression span also fails its dedicated test.

All three new tests and the existing debug/generated-source suites pass on CUDA 13.3, including assembler/GPU/CUDA-GDB. Strict Clippy for the new target and formatting pass. Combined source-location/debug-mode validation in #1613 passes all 35 tests. Fresh CI is running. This covers the short-circuit/folded-expression/const-bool portion of #1475; tracked in #1606.

@elibol elibol changed the title fix(cutile-compiler): attribute short-circuit boolean constants to expression span fix(cutile-compiler): preserve origins of short-circuit, folded and bool-binding constants Oct 26, 2026
@elibol
elibol marked this pull request as ready for review October 26, 2026 20:13
@elibol
elibol self-requested a review as a code owner October 26, 2026 20:13
elibol
elibol previously approved these changes Oct 28, 2026

@elibol elibol left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the current head. The short-circuit and folded-expression changes preserve the originating expression, and const-bool bindings use the callee definition rather than interpreting caller spans in the callee scope. The exact Line/Full and serialized cross-file caller-chain regressions cover the claimed frontend fixes; the combined focused suite also passes on CUDA 13.3. Approving this scope. Backend assertion-trap/coredump attribution remains tracked separately in #1379.

@elibol

elibol commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 701cf41

@elibol elibol left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed after merging current main and resolving the overlap with #1385. Both independent regression suites are retained, and the compiler diff remains limited to the previously reviewed short-circuit/folded/const-bool origins. All 21 focused constant/debug tests pass on CUDA 13.3, including assembly, GPU execution and live CUDA-GDB; formatting and diff checks pass. Renewing approval for this head.

GiGiKoneti and others added 3 commits October 28, 2026 17:02
…pression span

Short-circuit boolean operations (`a && b`, `a || b`) generate deciding
constants when the left-hand operand decides the outcome. Previously,
`compile_bool_constant` created constants from synthesized `bool` type
spans, resolving to fallback module-header locations.

Pass the logical expression's span through `compile_bool_constant` and the
shared constant builder (`compile_constant_from_exact_bounds`), ensuring
the resulting constant operation receives the exact expression line while
preserving inlined helper scopes and call chains.

Add regression test in `debug_info.rs` verifying source line attribution
and inline caller provenance for both direct and inlined expressions.

Part of NVIDIA#1475.

Signed-off-by: GiGiKoneti <gigikoneti@gmail.com>
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
…nance

Integrate current main. Attribute inlined boolean bindings to callee definitions instead of resolving caller coordinates in the callee module. Cover deciding false/true constants, a folded expression on a different line from its operands, and entry/function/method bindings in Line and Full modes with exact serialized inline chains.

Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
@elibol
elibol force-pushed the fix/cutile-short-circuit-spans branch from 701cf41 to 04ac9a4 Compare October 29, 2026 00:02

@elibol elibol left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renewing approval after the signature-only history rewrite. Every rewritten commit preserves its author and exact tree; the head tree is identical to 701cf41, where all 25 applicable checks passed, including Rust CodeQL and GPU CI. GitHub now verifies all three PR commits. No source or test change was made by signing. The earlier local 21-test assembler/GPU/GDB validation applies unchanged.

@elibol
elibol merged commit 42a1048 into NVIDIA:main Oct 29, 2026
elibol added a commit that referenced this pull request Oct 30, 2026
…1480)

Current review head: **`9f19c0c84e32e0b686154695b0f9bd96451b5d1f`**,
incorporating main through `0d62b682b` after #1528 and #1600 merged. The
diff against main remains four files and one source-policy change:
generated entry setup uses the user signature span.

Refresh validation: **36 tests passed** on CUDA 13.3 with serial
execution: 20 debug-info, three constant-origin, six generated-origin,
and seven JIT-override tests. This includes ignored assembler, GPU, and
live CUDA-GDB cases. Formatting and diff checks pass. Fresh CI is
running; independent review is still needed. The older validation
details below are historical evidence, not a claim that every suite was
rerun at this head.

Generated entry setup currently inherits `Span::call_site()`, so
CUDA-GDB attributes the wrapper and its generated assumptions to the
module header. Use the original user function's signature span for the
synthesized wrapper, including its call into the user kernel. The
original kernel body and nested helper locations remain unchanged.

The regression checks wrapper operations and assumptions, the outer
caller in inline chains, and the serialized entry subprogram's
declaration line. Constants compiled from synthesized types are outside
this change; #1385 has now merged their separate frontend-origin fix.
Backend assertion-trap/coredump attribution remains under #1379.

Validation:

- The new regression fails with the original wrapper span and passes
with this change.
- All nine `debug_info` tests pass, including real `tileiras`/`nvdisasm`
checks, execution in all debug modes, and the live cuda-gdb cross-file
helper test.
- Live CUDA-GDB confirms the wrapper frame and the entry instruction now
point to the user function signature.
- Scoped formatting and `cargo clippy --locked -p cutile-compiler --lib
-- -D warnings` pass.
- Ran the required `scripts/run_all.sh`: CPU/assembler tests, 26
supported examples, and all eight benchmark smoke tests passed. One
example was skipped because it requires Tile IR 13.4 and the installed
assembler is 13.3. All GPU targets passed except one existing test-order
dependency in `warmup_suite`; the whole 16-test cache suite passes
serially on this branch. The script therefore returned nonzero for that
unrelated cache test.

The unrelated cache failure is reproducible on unchanged base
`bafffe45662a5cdc064a1c6674c6c02d466a58e0`:
`retain_predicate_may_reenter_the_cache_without_deadlock` leaves the
same specialization cached, while
`composed_builder_compile_terminal_warms_the_dispatched_specialization`
expects its compile count to increase by one. Running the baseline test
binary with `RUSTC_BOOTSTRAP=1 <warmup_suite-binary> warmup_composed::
-Z unstable-options --shuffle-seed 3 --test-threads 1` reproduces that
assertion. This setting only enables the test harness's shuffle option;
it does not rebuild or change the pinned toolchain.

For benchmark validation, the existing runner drops its bare
`smoke-test` argument. For this run, its invocation was temporarily
corrected to `--features smoke-test` so all eight benchmarks used the
intended smoke configuration. The runner was restored afterward; that
correction is not part of the PR.

Part of #1475. The inline call-line breakpoint resolver is addressed
separately by
[NVIDIA/cuda-gdb#29](NVIDIA/cuda-gdb#29). The
generated assertion trap's backend source attribution remains under
#1379.

Signed-off-by: Melih Elibol <elibol@users.noreply.github.com>
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
Co-authored-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
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.

2 participants