-
Notifications
You must be signed in to change notification settings - Fork 0
Rewrite py.Mult to squin.Mult in squin kernel #212
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?
Conversation
CI fails because of kirin v0.17.3 (passes locally with 0.17.2). |
Looks like CI failure is caused by this commit: QuEraComputing/kirin@9cb1f5d The |
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.
you don't need to use fixpoint here because the termination condition is simple here - you walk once, all py.binop.Mult
is being replaced?
Also on 0.18 this will be simpler - we offer a dialect.canonicalize
decorator so you can register the rule only without implementing this Pass. We can come back and cleanup this part after I finish the full refactor.
@weinbe58 I'm wondering if you know what was causing the issue there? should we change the |
No, this is due to |
It was actually two issues, both of which you hinted at @Roger-luo :
One more question:
The Other than that: ready for review! |
028251c
to
858c4c0
Compare
Okay, so after rebasing the tests I added now fail due to kirin v0.17.3. Specifically, the type inference for The failing test checks to see if this kernel @squin.kernel
def scale_mult():
x = squin.op.x()
y = squin.op.y()
return 2 * (x * y) gets rewritten such that it includes exactly one kirin v0.17.2: scale_mult.print()
func.func scale_mult() -> !py.Op {
^0(%scale_mult_self):
│ %x = squin.op.x() : !py.Op
│ %y = squin.op.y() : !py.Op
│ %0 = py.constant.constant 2 : !py.int
│ %1 = squin.op.mult(lhs=%x, rhs=%y){is_unitary=False : !py.bool} : !py.Op
│ %2 = squin.op.scale(op=%1, factor=%0){is_unitary=False : !py.bool} : !py.Op
│ func.return %2
} // func.func scale_mult kirin v0.17.3: scale_mult.print()
func.func scale_mult() -> !py.int {
^0(%scale_mult_self):
│ %x = squin.op.x() : !py.Op
│ %y = squin.op.y() : !py.Op
│ %0 = py.constant.constant 2 : !Bottom
│ %1 = squin.op.mult(lhs=%x, rhs=%y){is_unitary=False : !py.bool} : !Bottom
│ %2 = py.binop.mult(%0, %1) : !py.int
│ func.return %2
} // func.func scale_mult @Roger-luo do you have an idea what's going on here? |
Co-authored-by: Phillip Weinberg <weinbe58@gmail.com>
Co-authored-by: Phillip Weinberg <weinbe58@gmail.com>
CI passes on v0.17.4 of kirin. Just need to wait for the release to be available. |
hmm 0.17.4 does not fix it? |
@Roger-luo that's a different error though. The new tests pass, maybe I just need another rebase to main. |
Seemed sufficiently different from #207, so I created a new branch for it.
@Roger-luo two things:
Fixpoint
.wire
kernel?