Skip to content

refactor: lower control flow to cf dialect instead of scf#20

Merged
tetsuo-cpp merged 1 commit intocanonfrom
unstructured-cf
Feb 19, 2026
Merged

refactor: lower control flow to cf dialect instead of scf#20
tetsuo-cpp merged 1 commit intocanonfrom
unstructured-cf

Conversation

@tetsuo-cpp
Copy link
Owner

Summary

  • Replace structured SCF control flow (scf.while, scf.for, scf.if) with unstructured CF branches (cf.br, cf.cond_br)
  • Remove region-based dialect ops (IfOp, BeginUntilOp, BeginWhileRepeatOp, DoLoopOp, YieldOp, LoopIndexOp) and add simpler PopFlagOp, PopOp, PushValueOp
  • Move control flow emission into the translator, enabling interleaved control flow (multi-WHILE, WHILE+UNTIL)

Related: #16

Copy link
Owner Author

@tetsuo-cpp tetsuo-cpp left a comment

Choose a reason for hiding this comment

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

Code Review

Clean, well-motivated refactoring. The move from SCF to CF is the right call — interleaved control flow patterns like multi-WHILE and WHILE+UNTIL genuinely can't be expressed with structured regions. The net result is a big simplification: ~340 lines of complex region manipulation code (convertRegionTypes, inlineRegionBefore, mergeBlocks) replaced by trivial PopFlagOp/PopOp/PushValueOp conversions, the isWhileLoop() lookahead hack eliminated, and one fewer pipeline stage.

Test coverage is solid — new interleaved CF tests at translation, conversion, and pipeline levels, plus GPU e2e tests.

Suggestions

cfStack lacks validation on pop: The pop_back_val() calls for ELSE, THEN, UNTIL, WHILE, REPEAT don't check the tag or guard against empty stack. A user writing IF REPEAT or THEN without IF would get a crash or garbage IR rather than a diagnostic. Only LOOP checks loopStack.empty(). Consider adding:

  • Empty checks before popping (like LOOP already does)
  • Tag assertions, e.g. assert(tag == CFTag::Dest) in REPEAT

DO/LOOP counter as alloca vs SSA: The memref<1xi64> alloca for the loop counter means every I/J/K access requires a memref.load. LLVM's mem2reg should optimize these, but worth keeping an eye on PTX output quality vs the old scf.for induction variable approach.

Minor: redundant arith.constant 0 : index emissions in check/body blocks for DO/LOOP — canonicalizer handles it, but could be hoisted.

Overall LGTM. The main action item would be defensive validation on cfStack pops.

@tetsuo-cpp tetsuo-cpp merged commit 2a78ff0 into canon Feb 19, 2026
1 check passed
@tetsuo-cpp tetsuo-cpp deleted the unstructured-cf branch February 19, 2026 04:55
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

Comments