Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Replaces the uses of quir call with the converted pulse call in QuirToPulse #273

Merged
merged 3 commits into from
Mar 7, 2024
Merged

Replaces the uses of quir call with the converted pulse call in QuirToPulse #273

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71eda16
copy over measure attributes
reza-j Feb 26, 2024
6d906b8
Merge branch 'main' into rj-copy-meas-pr
reza-j Mar 5, 2024
15208bb
address reviews
reza-j Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions include/Conversion/QUIRToPulse/QUIRToPulse.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ struct QUIRToPulsePass
mlir::Operation *mainFuncFirstOp;

// convert quir circuit to pulse sequence
void convertCircuitToSequence(mlir::quir::CallCircuitOp callCircuitOp,
mlir::func::FuncOp &mainFunc,
ModuleOp moduleOp);
mlir::pulse::CallSequenceOp
convertCircuitToSequence(mlir::quir::CallCircuitOp callCircuitOp,
mlir::func::FuncOp &mainFunc, ModuleOp moduleOp);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is one thing that confuses me about MLIR operations. The callCircuitOp and moduleOp are not explicitly passed by reference, but the mainFunc is.
Do you know if these are still being passed by reference, or if this is going to pass by value? Pass by value will create copies, which is inefficient. It is better to pass by reference and mark as const to indicate they will not be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested this today. There might be a better way to do this, but what I did was trying to print the memory location of variables before calling the function and inside the function with:

std::cout << &callCircuitOp << "\n"
std::cout << &moduleOp << "\n"

case1: function arguments are (mlir::quir::CallCircuitOp callCircuitOp, mlir::func::FuncOp &mainFunc, ModuleOp moduleOp)

case2: function arguments are (mlir::quir::CallCircuitOp &callCircuitOp, mlir::func::FuncOp &mainFunc, ModuleOp &moduleOp)

I noticed that in case1, the memory addresses were different before calling the function and inside the function, so my guess is that it copies in that case. While for case2, memory addressed were the same

given this, I added changes to have & in function args

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure if this is one of the cases where we can use const. I tried e.g., const mlir::quir::CallCircuitOp &callCircuitOp and got a compile error saying

error: 'this' argument to member function 'getOperand' has type 'const mlir::quir::CallCircuitOp', 
but function is not marked const
          auto durOpConstantOp = callCircuitOp.getOperand(argNum)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I see. MLIR seems to be missing a lot of const versions of the methods. I can see this would be painful to try and figure out what can be const as a result.
I think this is ok, we just need to be mindful that any changes that are made to the op now will persist (there shouldn't be any, but need to keep it in mind).

// helper datastructure for converting quir circuit to pulse sequence; these
// will be reset every time convertCircuitToSequence is called and will be
// used by several functions that are called within that function
Expand Down Expand Up @@ -149,10 +149,8 @@ struct QUIRToPulsePass
mlir::OpBuilder &builder);

void addCircuitToEraseList(mlir::Operation *op);
void addCallCircuitToEraseList(mlir::Operation *op);
void addCircuitOperandToEraseList(mlir::Operation *op);
std::vector<mlir::Operation *> quirCircuitEraseList;
std::vector<mlir::Operation *> quirCallCircuitEraseList;
std::vector<mlir::Operation *> quirCircuitOperandEraseList;

// parse the waveform containers and add them to pulseNameToWaveformMap
Expand Down
43 changes: 23 additions & 20 deletions lib/Conversion/QUIRToPulse/QUIRToPulse.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "Dialect/Pulse/IR/PulseOps.h"
#include "Dialect/Pulse/IR/PulseTypes.h"
#include "Dialect/QCS/IR/QCSOps.h"
#include "Dialect/QUIR/IR/QUIRDialect.h"
#include "Dialect/QUIR/IR/QUIREnums.h"
#include "Dialect/QUIR/IR/QUIROps.h"
#include "Dialect/QUIR/IR/QUIRTypes.h"
Expand Down Expand Up @@ -100,16 +101,15 @@ void QUIRToPulsePass::runOnOperation() {

// convert all QUIR circuits to Pulse sequences
moduleOp->walk([&](CallCircuitOp callCircOp) {
convertCircuitToSequence(callCircOp, mainFunc, moduleOp);
if (isa<CircuitOp>(callCircOp->getParentOp()))
return;
auto convertedPulseCallSequenceOp =
convertCircuitToSequence(callCircOp, mainFunc, moduleOp);
if (!callCircOp->use_empty())
callCircOp->replaceAllUsesWith(convertedPulseCallSequenceOp);
callCircOp->erase();
});

// first erase the quir call circuits
LLVM_DEBUG(llvm::dbgs() << "\nErasing quir call circuits:\n");
for (auto *op : quirCallCircuitEraseList) {
LLVM_DEBUG(op->dump());
op->erase();
}

// erase the quir circuits
LLVM_DEBUG(llvm::dbgs() << "\nErasing quir circuits:\n");
for (auto *op : quirCircuitEraseList) {
Expand All @@ -133,17 +133,17 @@ void QUIRToPulsePass::runOnOperation() {
});
}

void QUIRToPulsePass::convertCircuitToSequence(CallCircuitOp callCircuitOp,
mlir::func::FuncOp &mainFunc,
ModuleOp moduleOp) {
mlir::pulse::CallSequenceOp
QUIRToPulsePass::convertCircuitToSequence(CallCircuitOp callCircuitOp,
mlir::func::FuncOp &mainFunc,
ModuleOp moduleOp) {
mlir::OpBuilder builder(mainFunc);

auto circuitOp = getCircuitOp(callCircuitOp);
std::string const circName = circuitOp.getSymName().str();
LLVM_DEBUG(llvm::dbgs() << "\nConverting QUIR circuit " << circName << ":\n");
assert(callCircuitOp && "callCircuit op is null");
assert(circuitOp && "circuit op is null");
addCallCircuitToEraseList(callCircuitOp);
addCircuitToEraseList(circuitOp);

// build an empty pulse sequence
Expand Down Expand Up @@ -188,6 +188,15 @@ void QUIRToPulsePass::convertCircuitToSequence(CallCircuitOp callCircuitOp,
auto pulseCalCallSequenceOp =
entryBuilder.create<mlir::pulse::CallSequenceOp>(
quirOp->getLoc(), pulseCalSequenceOp, pulseCalSequenceArgs);

// copy the quir attributes of measure op (if any)
if (isa<MeasureOp>(quirOp)) {
for (auto attr : quirOp->getAttrs())
if (llvm::isa<mlir::quir::QUIRDialect>(attr.getNameDialect()))
pulseCalCallSequenceOp->setAttr(attr.getName().str(),
attr.getValue());
}

pulseCalCallSequenceOp->setAttr(
"pulse.operands",
pulseCalSequenceOp->getAttrOfType<ArrayAttr>("pulse.args"));
Expand Down Expand Up @@ -248,6 +257,8 @@ void QUIRToPulsePass::convertCircuitToSequence(CallCircuitOp callCircuitOp,
convertedPulseCallSequenceOp->setAttr(
"pulse.operands",
builder.getArrayAttr(convertedPulseCallSequenceOpOperandNames));

return convertedPulseCallSequenceOp;
}

void QUIRToPulsePass::processCircuitArgs(
Expand Down Expand Up @@ -651,14 +662,6 @@ void QUIRToPulsePass::addCircuitToEraseList(mlir::Operation *op) {
quirCircuitEraseList.push_back(op);
}

void QUIRToPulsePass::addCallCircuitToEraseList(mlir::Operation *op) {
assert(op && "caller requested adding a null op to erase list");
if (std::find(quirCallCircuitEraseList.begin(),
quirCallCircuitEraseList.end(),
op) == quirCallCircuitEraseList.end())
quirCallCircuitEraseList.push_back(op);
}

void QUIRToPulsePass::addCircuitOperandToEraseList(mlir::Operation *op) {
assert(op && "caller requested adding a null op to erase list");
if (std::find(quirCircuitOperandEraseList.begin(),
Expand Down
Loading