-
Notifications
You must be signed in to change notification settings - Fork 299
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
[Lower SCF/Standard to Calyx] Add support for scf.if
#4843
Comments
I'm less involved these days, but this makes sense to me. WDYT @mortbopet ? |
I think it would be a good idea, and i'd expect that the existing lowering for As you mention, there is no good way in MLIR to selectively ask conversion passes to only consider a subset of dialect operations (unless they explicitly implement this) which leads us into situations like this. |
scf.if
The original program is too simple and can therefore be canonicalized to func.func @main(%arg0 : i32, %arg1 : i32) -> i32 {
%0 = arith.cmpi slt, %arg0, %arg1 : i32
%1 = scf.if %0 -> i32 {
%3 = arith.addi %arg0, %arg1 : i32
scf.yield %3 : i32
} else {
scf.yield %arg1 : i32
}
return %1 : i32
} |
Hi folks, I'd like to give it a shot! |
@jiahanxie353 is a Cornell student and interested in working on this. We had a quick chat about possible next steps. Jiahan is going to look into the shape of the SCF to Calyx pass and come up with some concrete questions about the pattern rewriter infrastructure so that we can make progress. Also, for reference, here is the command that you need to run to trigger the error with the above program:
|
Hi folks, I've been diving deep into this, but hit a bit of a roadblock. I've created a draft PR to give you a snapshot of where I am, what I've been trying, and the challenges that have me scratching my head. |
I was wondering if it would be a good idea to add lowering support of
scf::IfOp
increateSCFToCalyxPass
.createSCFToCalyxPass
claims to handleBranchOpInterface
but when it encounters ascf::YieldOp
it asserts if it's notscf::WhileOp
.I've also tried using
createConvertSCFToCFPass
to lowerscf::IfOp
intocf
and it works well forscf::IfOp
, but the problem is that it also convertsscf::WhileOp
into acf::BranchOp
with back-edges, socreateSCFToCalyxPass
fails with:Therefore I think it's better to support
scf::IfOp
directly.Here's
simple_arith_if.mlir
, a port of simple_arith.mlir to usescf::IfOp
instead ofcf::CondBranchOp
, which can help test the lowering ofscf::IfOp
when implemented:(Thanks to @rachitnigam for the updated code.)
EDIT: You need to run the command:
The text was updated successfully, but these errors were encountered: