-
Notifications
You must be signed in to change notification settings - Fork 1
Fix AggressiveUnroll again #621
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
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
|
@cduck The problem here seems to be that the Somehow your change causes the inline to succeed, not sure why. |
|
Is there an issue to track this? I can dig a bit more on this since it's affecting downstream |
|
Oh, I think it might be as simple as this: @squin.kernel
def main(n: int):
def map_func(i):
return n + 1
return ilist.map(map_func, range(4))
main.print()
AggressiveUnroll(main.dialects).fixpoint(main)
main.print()
Edit: okay, this is really odd. If I change the |
This makes sense. Let's open an issue to fix AggressiveUnroll. I am confused how the line I added here changes the behavior, because it is a duplicate copy of the ilist.rewrite.Unroll pass but run earlier in the sequence. |
|
@cduck are you using |
Yes. Squin is using scf! |
|
For some reason typeinfer marked this as bottom: For some reason the final typeinfer rewrite in the UnrollScf pass modify that to Bottom |
|
My bad, accidentally closed the PR |
I see |
Do not merge until we understand the issue being fixed here. This became an issue again while upgrading my code for bloqade-circuit v0.9.
This pass has been unreliable before (see #593) and I'm not sure why adding a second copy of the
ilist.rewrite.Unrollpass fixes the issue.Here's the error it is fixing:
Tagging: @kaihsin