Skip to content

[Optimizer] Constant-fold cc::IfOp successor regions and invocation bounds#4191

Open
sanarang-nv wants to merge 3 commits intoNVIDIA:mainfrom
sanarang-nv:ifop-constant-fold
Open

[Optimizer] Constant-fold cc::IfOp successor regions and invocation bounds#4191
sanarang-nv wants to merge 3 commits intoNVIDIA:mainfrom
sanarang-nv:ifop-constant-fold

Conversation

@sanarang-nv
Copy link
Copy Markdown

**What does this PR implement? **

This PR applies two (similar) flavors of constant folding on the IfOp:

  • cc::IfOp::getSuccessorRegions: when the condition is a compile-time constant, only the live region is returned as a successor.
  • cc::IfOp::getRegionInvocationBounds: when the condition is constant, the live region gets
    exact bounds {1,1} (always executes) and the dead region gets {0,0} (never executes).

How do we test this?

  • C++ unit tests in unittests/Optimizer/IfOpConstantFoldTest.cpp directly call both methods
    with constant and non-constant operands and assert the correct regions/bounds are returned.
  • Lit test in test/Transforms/cc_if_const_fold.qke verifies end-to-end behavior

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot bot commented Mar 20, 2026

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.

@1tnguyen
Copy link
Copy Markdown
Collaborator

1tnguyen commented Mar 20, 2026

/ok to test 672d4b4

Command Bot: Processing...

@sacpis
Copy link
Copy Markdown
Collaborator

sacpis commented Mar 20, 2026

Seems like there are some build failures, please try to build cudaq locally using bash scripts/build_cudaq.sh -v

Signed-off-by: Samarth Narang <sanarang@nvidia.com>
@sanarang-nv
Copy link
Copy Markdown
Author

Seems like there are some build failures, please try to build cudaq locally using bash scripts/build_cudaq.sh -v

@sacpis, thanks. I wasn't able to setup the devcontainer earlier, but it went through and I was able to run build_cudaq successfully inside the container.

@sacpis
Copy link
Copy Markdown
Collaborator

sacpis commented Mar 20, 2026

/ok to test 32bf42c

Command Bot: Processing...

@github-actions
Copy link
Copy Markdown

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

@schweitzpgi
Copy link
Copy Markdown
Collaborator

I know folding IfOp was already done at one point. Let me see if I can find it...

@schweitzpgi
Copy link
Copy Markdown
Collaborator

schweitzpgi commented Mar 23, 2026

I know folding IfOp was already done at one point. Let me see if I can find it...

OK. There is a bit of code in the RewriteIf pattern in LowerToCFGPatterns.inc that already attempted to do this folding. We do not want both of them, as it will just be confusing. I think the solution in this PR is more in the spirit of MLIR, and would prefer it.

RewriteIf has a primary purpose of lowering If ops to a plain CFG (with branches), so it cannot be completely removed. Please remove the rewriteOnlyIfConst option from it however.

Nevermind, this does not really fold the cc.if.

Copy link
Copy Markdown
Collaborator

@schweitzpgi schweitzpgi left a comment

Choose a reason for hiding this comment

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

I want to check a few more things before approving this...

%c42 = arith.constant 42 : i32
cc.if (%true) {
func.call @use_i32(%c42) : (i32) -> ()
cc.continue
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.

Suggested change
cc.continue

This continue isn't necessary.

Copy link
Copy Markdown
Collaborator

@schweitzpgi schweitzpgi left a comment

Choose a reason for hiding this comment

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

Can you clarify what this is really affecting? To wit, for this input:

func.func private @use_i32(i32)

func.func @sink_into_true_branch() {
  %true = arith.constant true
  %c42 = arith.constant 42 : i32
  %c43 = arith.constant 43 : i32
  cc.if (%true) {
    func.call @use_i32(%c42) : (i32) -> ()
  } else {
    func.call @use_i32(%c43) : (i32) -> ()
  }
  return
}

The command line cudaq-opt -control-flow-sink test.qke without these changes will already produce.

module {
  func.func private @use_i32(i32)
  func.func @sink_into_true_branch() {
    %true = arith.constant true
    cc.if(%true) {
      %c42_i32 = arith.constant 42 : i32
      func.call @use_i32(%c42_i32) : (i32) -> ()
    } else {
      %c43_i32 = arith.constant 43 : i32
      func.call @use_i32(%c43_i32) : (i32) -> ()
    }
    return
  }
}

which suggests that this pass doesn't really exercise the changes.

Furthermore, the cc.if op is not folded in any way. If it neither removed nor is the else branch eliminated before or after these changes.

With the changes yields:

module {
  func.func private @use_i32(i32)
  func.func @sink_into_true_branch() {
    %true = arith.constant true
    cc.if(%true) {
      %c42_i32 = arith.constant 42 : i32
      func.call @use_i32(%c42_i32) : (i32) -> ()
    } else {
      %c43_i32 = arith.constant 43 : i32
      func.call @use_i32(%c43_i32) : (i32) -> ()
    }
    return
  }
}

which is the exact same IR.

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.

4 participants