Skip to content

Fix crash on non-empty return_void blocks#323

Open
ShangkunLi wants to merge 1 commit into
coredac:mainfrom
ShangkunLi:fix-cano-return
Open

Fix crash on non-empty return_void blocks#323
ShangkunLi wants to merge 1 commit into
coredac:mainfrom
ShangkunLi:fix-cano-return

Conversation

@ShangkunLi

Copy link
Copy Markdown
Collaborator

The --canonicalize-return pass previously assumed that every block containing a neura.return_void operation was an empty block (i.e., return_void was the only operation in it). When this assumption was violated — which happens for real-world kernels where other operations share the same basic block as return_void — the pass hit an unconditional assert(false) and crashed:

assert(false && "Unsupported case: return block is not empty.");

This made the pass fail on a broad class of benchmarks (fir, conv, spmv, jacobi, etc.) that produce non-empty return_void blocks after lowering.

Fix:

  • Removed the is_empty_block guard and the failing assert in both call sites inside processVoidReturnsInKernel and CanonicalizeReturnPass.
  • Renamed processEmptyReturnVoidBlockprocessReturnVoidBlock to reflect that it now handles the general case.
  • The underlying predecessor-walking logic already handled non-empty blocks correctly; the guard was overly conservative.

@ShangkunLi ShangkunLi self-assigned this Jun 10, 2026
@ShangkunLi ShangkunLi added the bug Something isn't working label Jun 10, 2026

@tancheng tancheng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we don't need to update any tests? They all already passed?

Comment on lines -346 to -349
// TODO: Handle non-empty return blocks.
// The basic idea is to create a new block that only contains the
// return_void operation, and redirect the original return block to
// this new block.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we previously want a standalone bb only containing return? Can you recall?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants