refactor: lower control flow to cf dialect instead of scf

[tetsuo-cpp]

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

[tetsuo-cpp]

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]

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

[tetsuo-cpp]

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]

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

[tetsuo-cpp]

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]

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

[tetsuo-cpp]

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]

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

[tetsuo-cpp]

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]

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

[tetsuo-cpp]

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]

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

[tetsuo-cpp]

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]

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

[tetsuo-cpp]

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.