Make sure uccsd from cudaqx compiles for quantum devices #2458
Conversation
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
…x-for-demo Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
|
CUDA Quantum Docs Bot: A preview of the documentation can be found here. |
…x-for-demo Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
…x-for-demo Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
I, Anna Gringauze <agringauze@nvidia.com>, hereby add my Signed-off-by to this commit: ae35d16 Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
|
CUDA Quantum Docs Bot: A preview of the documentation can be found here. |
There was a problem hiding this comment.
I don't like the idea of making the patterns reside in files other than the pass where they are applied. That's not been the practice. So I think we should merge them back.
The exception to this general rule is when patterns are shared in multiple passes, which these are not AFAICT. Instead of block copying the same pattern in 2 or more different passes and where they will become hopelessly out of synch with one another, we want to put the pattern in an include file (foo.inc) and include that file in the n passes where it is needed. The patterns in LowerToCFG for IfOp is such a case, since it will be the same rewrite in LowerToCFG (indiscriminate) and in the canonicalization pattern you want to add (selected only when the condition is constant).
| // Prevent quantum allocations from moving to an outer scope. | ||
| if (block && hasQuantumAllocations(*block)) | ||
| return failure(); |
There was a problem hiding this comment.
Was there an example where this happened? If so, the IR has bugs.
Any allocations must be either in a basic block owned by the func::FuncOp, cc::LambdaCreateOp, or a cc::ScopeOp. So this check isn't needed for valid IR.
| if (auto cont = dyn_cast<cudaq::cc::ContinueOp>(block->back())) { | ||
| for (std::size_t i = 0; auto opnd : ifOp.getResults()) | ||
| rewriter.replaceAllUsesWith(opnd, cont.getOperand(i++)); | ||
| rewriter.eraseOp(cont); |
There was a problem hiding this comment.
There is code in LowerToCFG.cpp that folds away cc::IfOps. I think you can use the RewriteIf pattern directly from there (share it in an .inc file). Basically, we would use that pattern on any IfOp with a constant argument. Using the pattern will get the linear arguments propagated correctly. Also, since the argument is a constant, DCE will simply clean things up and fuse the blocks, etc.
| CollapseStores.cpp | ||
| CollapseStoresPatterns.cpp |
There was a problem hiding this comment.
It looks like these ought to be combined into 1 file. We put the patterns in the same source file with the pass.
| LiftArrayAlloc.cpp | ||
| LiftArrayAllocPatterns.cpp |
There was a problem hiding this comment.
I'm not sure why these are being split into two files in this PR?
…x-for-demo Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
|
CUDA Quantum Docs Bot: A preview of the documentation can be found here. |
|
CUDA Quantum Docs Bot: A preview of the documentation can be found here. |
Description
Fix a number of issues with compiler passes discovered in
uccsdcompilation for quantum devices.Details
uccsd: Update to not contain nested arraysast_bridge: Fix incorrect boundaries generated forrangecalls.LiftArrayAllocFix crashes and incorrectly optimizing multiple stores into the same pointerCollapseStores: Remove useless storesif(%true)andif(%false)statementsLoopNormalize: handle negative boundariesLoopUnroll: Add other classic optimizations to run alongLoopUnroll:LoopNormalizeLiftArrayAllocCollapseStoressimplifyRegionsRelated: NVIDIA/cudaqx#64
Requires: #2570
Requires: #2571Requires: #2572
Requires: #2573