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

P4 spec issue 1261 add bounded loops #4120

Conversation

jafingerhut
Copy link
Contributor

No description provided.

@jafingerhut jafingerhut marked this pull request as draft August 27, 2023 21:03
@jafingerhut
Copy link
Contributor Author

This is NOT a complete implementation of for loops in p4c. It probably isn't even a complete or correct change to the syntax as of the first 2 commits. It is a work in progress that I have used to verify that with these proposed grammar changes, bison does not give an error message when it processes the grammar.

@asl
Copy link
Contributor

asl commented Mar 5, 2024

@jafingerhut Some time ago we implemented bounded loops downstream. They employ for (variable in sequence) syntax as a way to enforce that the loop is indeed bounded (it seems quite a non-trivial task in general for C-style for (start; end; increment) loops).

Let me know if you'd like me to factor our this change and submit just to have some end-to-end implementation.

@fruffy
Copy link
Collaborator

fruffy commented Mar 5, 2024

Bounded loops are a top-level agenda item of the P4 language working group for this year:

https://docs.google.com/document/d/1XSgdXeG1UuF1FM_XAqxDrHeN4dHZWBnJPKVS6SnGNwM/edit#heading=h.q7bzcwglv61g

Upstreaming this is a great idea and very useful. How the syntax will end up looking like is a different problem though.

@asl
Copy link
Contributor

asl commented Mar 5, 2024

Sure, I can see what could be done.

@jafingerhut
Copy link
Contributor Author

@asl Thanks for the offer. Fabian gave the high order answer. There should be a public meeting focused on the topic of loops in P4 coming up soon-ish (within a month or so), and I can be sure to try to include you in the choice of meeting time so it fits in your schedule.

@fruffy fruffy added the p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/). label Mar 15, 2024
Copy link
Contributor

@ChrisDodd ChrisDodd left a comment

Choose a reason for hiding this comment

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

I've started looking at loops in the compiler, so I'm looking at what to add in the IR for these, and what is needed in other parts of the compiler.

forCollectionExpr
: expression
| expression ".." expression
| typeRef
Copy link
Contributor

Choose a reason for hiding this comment

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

What is a typeRef supposed to indicate here? Creating a temp of the type and then iterating over it?

@ChrisDodd
Copy link
Contributor

I added an extra commit with the IR changes for this.

@asl
Copy link
Contributor

asl commented Mar 25, 2024

I added an extra commit with the IR changes for this.

I've almost finished porting our changes for the loops. I will post a PR shortly

@asl
Copy link
Contributor

asl commented Mar 25, 2024

I ported our downstream code at #4558

Currently use-def changes are missed, I am working on porting them asap.

Let me know if there are any questions / comments / suggestions

@ChrisDodd ChrisDodd mentioned this pull request Mar 26, 2024
@ChrisDodd
Copy link
Contributor

I've pulled together both changes in #4562

@asl
Copy link
Contributor

asl commented Mar 26, 2024

I was not part of the original discussion, so do not know about all the details here. But my understanding is that if the intention is to support bounded loops, then I think it should iterate over collection (or as in my original PR, over explicit range like a ... b). Generic C-style loops for (start; cond; update) are much harder to prove that they are bounded.

@jafingerhut
Copy link
Contributor Author

Closing this preliminary PR, as #4562 and other changes by Chris Dodd have been merged in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants