Update canonicalization of cc.if - #5303
Conversation
Signed-off-by: Adam Geller <adgeller@nvidia.com>
CI Summary (
|
| 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 |
| // General case: the region has multiple exits, so split the parent block at | ||
| // the cc.if and stitch the region in with branches. | ||
| auto *ifBlock = rewriter.getInsertionBlock(); | ||
| auto *splitBlock = | ||
| rewriter.splitBlock(ifBlock, rewriter.getInsertionPoint()); | ||
| Block *succBlock = splitBlock; | ||
| if (ifOp.getNumResults()) { | ||
| succBlock = rewriter.createBlock( | ||
| splitBlock, ifOp.getResultTypes(), | ||
| SmallVector<Location>(ifOp.getNumResults(), loc)); | ||
| cf::BranchOp::create(rewriter, loc, splitBlock); | ||
| } | ||
| rewriter.replaceOpWithNewOp<cudaq::cc::IfOp>( | ||
| ifOp, ifOp.getResultTypes(), truth, | ||
| [&](OpBuilder &, Location, Region ®ion) { | ||
| region.takeBody(*newRegion); | ||
| }); | ||
| auto *entryBlock = ®ion.front(); | ||
| for (auto &block : region) | ||
| if (auto contOp = | ||
| dyn_cast<cudaq::cc::ContinueOp>(block.getTerminator())) { | ||
| rewriter.setInsertionPointToEnd(&block); | ||
| rewriter.replaceOpWithNewOp<cf::BranchOp>(contOp, succBlock, | ||
| contOp.getOperands()); | ||
| } | ||
| rewriter.inlineRegionBefore(region, succBlock); | ||
| rewriter.setInsertionPointToEnd(ifBlock); | ||
| cf::BranchOp::create(rewriter, loc, entryBlock, ifOp.getLinearArgs()); | ||
| rewriter.replaceOp(ifOp, succBlock->getArguments()); | ||
| return success(); |
There was a problem hiding this comment.
If we fold a multi exit cc.if inside a loop's while or step region it produces invalid IR and the canonicalizer errors out.
I tried running cudaq-opt -canonicalize on the following mlir
func.func private @side() -> i1
func.func @in_while(%arg: i1) {
%c = arith.constant true
cc.loop while {
cc.if (%c) {
cf.cond_br %arg, ^bb1, ^bb2
^bb1:
%z = func.call @side() : () -> i1
cc.continue
^bb2:
cc.continue
}
%p = func.call @side() : () -> i1
cc.condition %p
} do {
%y = func.call @side() : () -> i1
cc.continue
}
return
}
It gives me the error as
error: 'cc.loop' op region #0 ('whileRegion') failed to verify constraint: region with 1 blocks
I think before taking the multi exit path, we should bail out when the region holding the cc.if cannot take extra blocks. The single block path do not need such a guard.
schweitzpgi
left a comment
There was a problem hiding this comment.
Thanks for the renovations. A couple of quick comments.
| PatternRewriter &rewriter) const override { | ||
| auto cond = ifOp.getCondition(); | ||
| auto con = cond.getDefiningOp<arith::ConstantIntOp>(); | ||
| auto con = ifOp.getCondition().getDefiningOp<arith::ConstantIntOp>(); |
There was a problem hiding this comment.
Let's use cudaq::opt::factory::maybeValueOfIntConstant
| rewriter.inlineBlockBefore(&entryBlock, ifOp, ifOp.getLinearArgs()); | ||
| // Simple case: a single block, which must end with a cc.continue. Splice | ||
| // the block in place of the cc.if. | ||
| if (region.hasOneBlock()) { |
There was a problem hiding this comment.
Can we splice the entire region, rewrite the cc.continue operations to cf.br to a new block with block arguments and then rewrite all uses of the cc.if values to use those block arguments? That should handle the CFG inside the then/else region cases.
It should be the same, effectively, as what lower-to-cfg does. MIght be a refactor opportunity?
| } | ||
| }; | ||
|
|
||
| struct KillIfWithNoOpArms : public OpRewritePattern<cudaq::cc::IfOp> { |
No description provided.