Fixing the Python bridge's loop and scope emission

[sacpis]

Follow-up to #5233. @schweitzpgi listed several problems with the IR the bridge emits for a nested-loop kernel. This fixes them and removes the two workarounds #5233 added.

• cc.scope for compound statements. The bridge emitted none, so add-deallocs had nothing to attach to and a qubit allocated in an if branch or a loop body was never deallocated.
• for loops in memory form. The bridge lifted the counter to an iteration argument itself. Now each for holds a counter slot, loaded to test and load/increment/stored to step. The counter is separate from the Python loop variable, since assigning that variable in the body must not change the trip count.
• New cc-loop-prune-dead-args pass. memtoreg promotes an entry-block local to a value carried by every enclosing loop, read or not, which blocks adjoint generation. The pass drops those slots. Nested loops hold each other's alive, so liveness starts from assume dead and runs to a fixpoint.

This lets both #5233 workarounds go. The bridge no longer allocates loop variables in the loop body, and variable-coalesce no longer skips allocas under a loop.

• Result
  Python locals are back in the entry block, no bare allocas, no pre-lifted counters, and the QFT compiles with zero dead loop-carried values.

[github-actions]

CI Summary (push) — ✅ passed

Run #33038828838 · ✅ 7 · ⏩ 7 · ❌ 0 · ⛔ 0

Top-level jobs (14)

Job	Result
binaries	⏩ skipped
build_and_test	✅ success
changes	✅ success
config_devdeps	✅ success
config_source_build	⏩ skipped
config_wheeldeps	✅ success
devdeps	✅ success
docker_image	⏩ skipped
gen_code_coverage	⏩ skipped
metadata	✅ success
python_metapackages	⏩ skipped
python_wheels	⏩ skipped
source_build	⏩ skipped
wheeldeps	✅ success

⏩ Skipped jobs (7) — intentionally skipped on PR builds; run on merge_group / workflow_dispatch

Job
binaries
config_source_build
docker_image
gen_code_coverage
python_metapackages
python_wheels
source_build

All sub-jobs (43) — every matrix leg, with links

Job	Status	Link
Build and test (amd64, gcc12, openmpi) / Dev environment (Debug)	✅ success	view
Build and test (amd64, gcc12, openmpi) / Dev environment (Python)	✅ success	view
Build and test (amd64, llvm, openmpi) / Dev environment (Debug)	✅ success	view
Build and test (amd64, llvm, openmpi) / Dev environment (Python)	✅ success	view
Build and test (arm64, llvm, openmpi) / Dev environment (Debug)	✅ success	view
Build and test (arm64, llvm, openmpi) / Dev environment (Python)	✅ success	view
CI Summary	❔ in_progress	view
Check for stable CUDA-Q changes	✅ success	view
Configure build (devdeps)	✅ success	view
Configure build (source_build)	⏩ skipped	view
Configure build (wheeldeps)	✅ success	view
Create CUDA Quantum installer	⏩ skipped	view
Create Docker images	⏩ skipped	view
Create Python metapackages	⏩ skipped	view
Create Python wheels	⏩ skipped	view
Gen code coverage	⏩ skipped	view
Load dependencies (amd64, gcc12) / Caching	✅ success	view
Load dependencies (amd64, gcc12) / Finalize	✅ success	view
Load dependencies (amd64, gcc12) / Metadata	✅ success	view
Load dependencies (amd64, llvm) / Caching	✅ success	view
Load dependencies (amd64, llvm) / Finalize	✅ success	view
Load dependencies (amd64, llvm) / Metadata	✅ success	view
Load dependencies (arm64, gcc12) / Caching	✅ success	view
Load dependencies (arm64, gcc12) / Finalize	✅ success	view
Load dependencies (arm64, gcc12) / Metadata	✅ success	view
Load dependencies (arm64, llvm) / Caching	✅ success	view
Load dependencies (arm64, llvm) / Finalize	✅ success	view
Load dependencies (arm64, llvm) / Metadata	✅ success	view
Load source build cache	⏩ skipped	view
Load wheel dependencies (amd64, 12.6) / Caching	✅ success	view
Load wheel dependencies (amd64, 12.6) / Finalize	✅ success	view
Load wheel dependencies (amd64, 12.6) / Metadata	✅ success	view
Load wheel dependencies (amd64, 13.0) / Caching	✅ success	view
Load wheel dependencies (amd64, 13.0) / Finalize	✅ success	view
Load wheel dependencies (amd64, 13.0) / Metadata	✅ success	view
Load wheel dependencies (arm64, 12.6) / Caching	✅ success	view
Load wheel dependencies (arm64, 12.6) / Finalize	✅ success	view
Load wheel dependencies (arm64, 12.6) / Metadata	✅ success	view
Load wheel dependencies (arm64, 13.0) / Caching	✅ success	view
Load wheel dependencies (arm64, 13.0) / Finalize	✅ success	view
Load wheel dependencies (arm64, 13.0) / Metadata	✅ success	view
Prepare cache clean-up	❔ in_progress	view
Retrieve PR info	✅ success	view

✅ Required checks (6/6) — declared in .github/required-checks.yml for push

Required check	Status	Link
Build and test (amd64, llvm, openmpi) / Dev environment (Debug)	✅ success	view
Build and test (amd64, llvm, openmpi) / Dev environment (Python)	✅ success	view
Build and test (arm64, llvm, openmpi) / Dev environment (Debug)	✅ success	view
Build and test (arm64, llvm, openmpi) / Dev environment (Python)	✅ success	view
Build and test (amd64, gcc12, openmpi) / Dev environment (Debug)	✅ success	view
Build and test (amd64, gcc12, openmpi) / Dev environment (Python)	✅ success	view

[schweitzpgi]

This was going to be removed, eh?

[sacpis]

This pass is what lets two workarounds on main go away. Both are from #5233.

1. the alloca sinking in visit_For / __analyzeLoopLocalTargets (the bare cc.alloca in the loop body)
2. the isNestedInLoop guard in VariableCoalesce.cpp, which refuses to raise any alloca nested in a cc.loop

This branch deletes both and prunes the dead loop-carried values in the optimizer instead. #5223 needs one of the three. With the pass disabled and nothing else changed, test_5223.py fails with the original 'arith.addi' op control-flow def-use not reversible.

[schweitzpgi]

OK.

1. Random stray alloca's inside high-level control-flow operations without scopes are 100% invalid IR from the front-end. The Python bridge needs to generate correct IR.
2. The variable-coalesce pass doesn't move allocations. That's not what it is for. It merges variables based on their lifetimes. The stack-frame-prealloc pass is the pass that moves allocations to the prologue of the function. These are complimentary but orthogonal.

So the isNestedInLoop stuff to move cc.alloca around is misplaced and possibly redundant with what stack-frame-prealloc does.

[sacpis]

Totally agreed on the front end, and that's what this PR does. The bridge emits cc.scope for if/else and loop bodies now, and the sinking that produced the bare cc.alloca is gone.

The isNestedInLoop guard was a consequence of that same front end bug. variable-coalesce says it relies on the correct and proper construction and use of cc.scope ops, and the bridge emitted none, so #5233 guarded the pass instead of fixing the IR. With scopes emitted the guard is not needed, and coalesce.qke's f2 goes back to 2 slots from 4.

stack-frame-prealloc is downstream of this (it runs in addLowerToCFGAndCleanup after lowering to CFG, whereas apply-op-specialization runs on the high-level IR, so it cannot cover this case).

[schweitzpgi]

Still looks wrongheaded.

[sacpis]

Fixed in 07324e6. The sweep is gone. The binding update is now anchored to the store being rewritten. If the user we're rewriting is a cc.store of this def's value, we rebind only that store's target. No search by value identity, so it touches exactly one variable rather than every binding that happens to match.

Note: This one is a miscompile on main today. j = i in a loop body reads i's pre-loop value, in Python and in C++ through nvq++ alike. Happy to split it into its own PR.

[sacpis]

Here is the qke

func.func @repro(%n: i64) -> i64 {
  %c0 = arith.constant 0 : i64
  %c1 = arith.constant 1 : i64
  %ctr = cc.alloca i64
  %v = cc.alloca i64
  cc.store %c0, %ctr : !cc.ptr<i64>
  cc.store %c0, %v : !cc.ptr<i64>
  cc.loop while {
    %a = cc.load %ctr : !cc.ptr<i64>
    %c = arith.cmpi slt, %a, %n : i64
    cc.condition %c
  } do {
    %a = cc.load %ctr : !cc.ptr<i64>
    cc.store %a, %v : !cc.ptr<i64>
    cc.continue
  } step {
    %a = cc.load %ctr : !cc.ptr<i64>
    %b = arith.addi %a, %c1 : i64
    cc.store %b, %ctr : !cc.ptr<i64>
  }
  %r = cc.load %v : !cc.ptr<i64>
  return %r : i64
}

after running cudaq-opt --memtoreg=quantum=0 repro.qke

module {
  func.func @repro(%arg0: i64) -> i64 {
    %c0_i64 = arith.constant 0 : i64
    %c1_i64 = arith.constant 1 : i64
    %0 = cc.undef i64
    %1 = cc.undef i64
    %2:2 = cc.loop while ((%arg1 = %c0_i64, %arg2 = %c0_i64) -> (i64, i64)) {
      %3 = arith.cmpi slt, %arg2, %arg0 : i64
      cc.condition %3(%arg1, %arg2 : i64, i64)
    } do {
    ^bb0(%arg1: i64, %arg2: i64):
      cc.continue %c0_i64, %arg2 : i64, i64
    } step {
    ^bb0(%arg1: i64, %arg2: i64):
      %3 = arith.addi %arg2, %c1_i64 : i64
      cc.continue %arg1, %3 : i64, i64
    }
    return %2#0 : i64
  }
}

Please see cc.continue %c0_i64, %arg2 in the do region. It carries the value the counter had before the loop, so the function returns 0 for any n > 0.

With this fix on this branch, we get cc.continue %arg2, %arg2.

With this code

int i = 0, j = 0;
while (i < 3) {
    j = i;
    i = i + 1;
}
x(q[j]);

The kernel above set j to 2.
So when we run the snippet on main, it gives

{ 1000:10 }

This fix gives the correct output as

{ 0010:10 }

Hope this helps.

[schweitzpgi]

I'd like to see the fixes for Python for-else pulled out into a completely separate PR.

This seems to be a grab bag of random changes. Changes to enable block argument elimination (that apparently don't work) and coupled with an entirely new pass to do that work instead. And, it's not clear any of it is needed at all as the original motivating example can have its adjoint manufactured without those changes.

The Python bridge fixes are mixed in as well when they are again a separate issue entirely and should be another separate PR.

This PR is just too scattered and trying to lump bug fixes with new features across the entire code base.