-
Notifications
You must be signed in to change notification settings - Fork 447
Add option to control unwind lowering #5274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
037eb93
7d4fdfc
572f203
b76dbbf
8b2131f
2f952fb
77314c6
5f190f5
7fe732b
1ed24e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -160,16 +160,18 @@ class CheckKernelCalls | |
| static void defaultInlinerOptPipeline(OpPassManager &pm) {} | ||
|
|
||
| /// Run the passes in the correct order. | ||
| /// 1) Lower unwind control flow before creating call-site scopes. | ||
| /// 1) Optionally lower unwind control flow before creating call-site scopes. | ||
| /// 2) Convert calls between kernels to direct calls (on the QPU). | ||
| /// 3) Aggressively inline all calls. | ||
| /// 4) Detect if kernel inlining has failed and left behind calls to kernels. | ||
| /// Such a failure is most likely a sign that there is a cycle in the call | ||
| /// graph. [This check is a bad idea: this should be deferred to final codegen | ||
| /// when translating the final Quake IR.] | ||
| void cudaq::opt::addAggressiveInlining(OpPassManager &pm, bool fatalChecks) { | ||
| void cudaq::opt::addAggressiveInlining(OpPassManager &pm, bool fatalChecks, | ||
| bool lowerUnwind) { | ||
| llvm::StringMap<OpPassManager> opPipelines; | ||
| pm.addNestedPass<func::FuncOp>(cudaq::opt::createUnwindLowering()); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this is truly weird. Unwinding ought to happen very early. Once done, it should never be thought about again. It's a pass for lowering very high-level syntactic goop from the front-end to a correct and selectable lower-level IR. That selectability is precisely what we want now. |
||
| if (lowerUnwind) | ||
| pm.addNestedPass<func::FuncOp>(cudaq::opt::createUnwindLowering()); | ||
| pm.addPass(cudaq::opt::createConvertToDirectCalls()); | ||
| pm.addPass(createInlinerPass(opPipelines, defaultInlinerOptPipeline)); | ||
| if (fatalChecks) | ||
|
|
@@ -192,6 +194,10 @@ struct AggressiveInliningPipelineOptions | |
| *this, "fatal-check", | ||
| llvm::cl::desc("run checker and produce fatal errors immediately"), | ||
| llvm::cl::init(false)}; | ||
| PassOptions::Option<bool> lowerUnwind{ | ||
| *this, "lower-unwind", | ||
| llvm::cl::desc("lower unwind operations before inlining"), | ||
| llvm::cl::init(true)}; | ||
| }; | ||
| } // namespace | ||
|
|
||
|
|
@@ -200,6 +206,6 @@ void cudaq::opt::registerAggressiveInliningPipeline() { | |
| "aggressive-inlining", | ||
| "Convert calls between kernels to direct calls and inline functions.", | ||
| [](OpPassManager &pm, const AggressiveInliningPipelineOptions &opt) { | ||
| addAggressiveInlining(pm, opt.runFatalChecker); | ||
| addAggressiveInlining(pm, opt.runFatalChecker, opt.lowerUnwind); | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| // ========================================================================== // | ||
| // Copyright (c) 2026 NVIDIA Corporation & Affiliates. // | ||
| // // | ||
| // This source code and the accompanying materials are made available under // | ||
| // the terms of the Apache License 2.0 which accompanies this distribution. // | ||
| // ========================================================================== // | ||
|
|
||
| // RUN: cudaq-opt --dump-pass-pipeline \ | ||
| // RUN: --pass-pipeline='builtin.module(python-aot-pipeline{unwind-mode=dataflow})' \ | ||
| // RUN: %s 2>&1 | FileCheck %s --check-prefix=DATAFLOW | ||
| // RUN: cudaq-opt --dump-pass-pipeline \ | ||
| // RUN: --pass-pipeline='builtin.module(python-aot-pipeline{unwind-mode=cfg})' \ | ||
| // RUN: %s 2>&1 | FileCheck %s --check-prefix=CFG | ||
| // RUN: cudaq-opt --dump-pass-pipeline \ | ||
| // RUN: --pass-pipeline='builtin.module(python-aot-pipeline{unwind-mode=none})' \ | ||
| // RUN: %s 2>&1 | FileCheck %s --check-prefix=NONE | ||
| // RUN: not cudaq-opt \ | ||
| // RUN: --pass-pipeline='builtin.module(python-aot-pipeline{unwind-mode=invalid})' \ | ||
| // RUN: %s 2>&1 | FileCheck %s --check-prefix=INVALID | ||
| // RUN: not cudaq-opt \ | ||
| // RUN: --pass-pipeline='builtin.module(python-aot-pipeline{unwind-mode=none},lower-to-cfg)' \ | ||
| // RUN: %s 2>&1 | FileCheck %s --check-prefix=NONE-ERROR | ||
|
|
||
| module { | ||
| func.func @early_return(%arg0: i1) -> i32 { | ||
| %c1 = arith.constant 1 : i32 | ||
| cc.if(%arg0) { | ||
| cc.unwind_return %c1 : i32 | ||
| } | ||
| %c0 = arith.constant 0 : i32 | ||
| return %c0 : i32 | ||
| } | ||
| } | ||
|
|
||
| // DATAFLOW: Pass Manager with 28 passes: | ||
| // DATAFLOW: variable-coalesce | ||
| // DATAFLOW: unwind-by-dataflow | ||
| // DATAFLOW: canonicalize | ||
|
|
||
| // CFG: Pass Manager with 28 passes: | ||
| // CFG: variable-coalesce | ||
| // CFG: unwind-lowering | ||
| // CFG: canonicalize | ||
|
|
||
| // NONE: Pass Manager with 26 passes: | ||
| // NONE-NOT: unwind-lowering | ||
| // NONE-NOT: unwind-by-dataflow | ||
| // NONE: get-concrete-matrix | ||
|
|
||
|
|
||
| // INVALID: Cannot find option named 'invalid' | ||
| // NONE-ERROR: error: 'cc.unwind_return' op |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are separate passes that ought to have no relationship to each other. I'm not sure why unwinding was buried in the inlining pipeline. It should be pulled out and stand on its own feet at this point.