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.

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

feat(llvm): Tail loop emission #1749

Merged
merged 7 commits into from
Dec 11, 2024
Merged

feat(llvm): Tail loop emission #1749

merged 7 commits into from
Dec 11, 2024

Conversation

croyzor
Copy link
Contributor

@croyzor croyzor commented Dec 10, 2024

Work in progress. Will address #1688

@croyzor
Copy link
Contributor Author

croyzor commented Dec 10, 2024

I think the problem with this code (the reason it's failing the simple test anyway) is that it's not strict enough about when we should be expecting the data to be packed in a sum type or not.

  • Entry to the whole thing should expect #I, #X
  • Entry to the loop should also expect #I, #X
  • The loop body should return Sum(#I + #O), #X
  • The postprocessing step should then unpack the data and call either the "continue" branch or "break" branch. Hence it expects the Sum(#I + #O), #X emitted by the loop body
  • Hence, the "continue" branch should expect #I, #X
  • And the "break" branch should expect #O, #X
  • The exit from the whole tail loop should be #O, #X
  • I.e. the signature of the tail loop is #I, #X -> #O, #X

(Using terminology from the spec)

@doug-q doug-q force-pushed the feat/tail-loop-emission branch from 4bad836 to 275534b Compare December 11, 2024 06:38
@ss2165 ss2165 self-assigned this Dec 11, 2024
@ss2165 ss2165 force-pushed the feat/tail-loop-emission branch 2 times, most recently from 92927ec to c179e0a Compare December 11, 2024 11:35
@ss2165 ss2165 marked this pull request as ready for review December 11, 2024 14:24
@ss2165 ss2165 requested a review from a team as a code owner December 11, 2024 14:24
@ss2165 ss2165 self-requested a review December 11, 2024 14:24
@ss2165 ss2165 changed the title WIP: Tail loop emission feat(llvm): Tail loop emission Dec 11, 2024
@ss2165 ss2165 force-pushed the feat/tail-loop-emission branch from 2b8fd1b to 7bcb9eb Compare December 11, 2024 14:24
@ss2165 ss2165 requested a review from doug-q December 11, 2024 14:30
Copy link
Collaborator

@doug-q doug-q left a comment

Choose a reason for hiding this comment

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

Thanks!

hugr-llvm/src/emit/ops.rs Outdated Show resolved Hide resolved
hugr-llvm/src/emit/test.rs Outdated Show resolved Hide resolved
Co-authored-by: Douglas Wilson <[email protected]>
@ss2165 ss2165 enabled auto-merge December 11, 2024 15:52
@ss2165 ss2165 added this pull request to the merge queue Dec 11, 2024
Merged via the queue into main with commit 89545e8 Dec 11, 2024
19 checks passed
@ss2165 ss2165 deleted the feat/tail-loop-emission branch December 11, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants