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

standard annotations on extern functions/methods #802

Closed
wants to merge 2 commits into from

Conversation

ChrisDodd
Copy link
Contributor

No description provided.

Copy link
Collaborator

@jnfoster jnfoster left a comment

Choose a reason for hiding this comment

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

I wonder if we should try to rationalize these names a bit and perhaps signal somehow that they are for externs? I don't have any brilliant ideas, but I will observe that right now it's not clear that pure and packetState are related in any way. It could be nice if that were the case.

not depend on any hidden state that may be changed by other calls.
An example is the `hash` function which computes a deterministic
hash of its arguments. A `@pure` function whose results are unused
may be safely eliminated with no adverse effects, and mulitple calls
Copy link
Collaborator

Choose a reason for hiding this comment

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

"multiple"

hash of its arguments. A `@pure` function whose results are unused
may be safely eliminated with no adverse effects, and mulitple calls
with identical arguments may be combined into a single call. `@pure`
functions may also be reoerdered with respect to other computations
Copy link
Collaborator

Choose a reason for hiding this comment

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

"reordered"

@jnfoster
Copy link
Collaborator

jnfoster commented Dec 31, 2019

I liked how @pure had a simple example of a compiler optimization associated with it. Could we do the same for the other annotations?

@jafingerhut
Copy link
Collaborator

I do not know if there are any useful compiler optimizations that can be made with the localState, packetState, and localIndexedState annotations. My only reason for introducing them was noticing that there are some fairly consistent restrictions of these kinds on most of the externs in v1model and PSA architectures.

Some kind of name prefix would be one way to make it more obvious that these annotations are related to one another.

@jafingerhut jafingerhut reopened this Dec 31, 2019
- `@noSideEffects` - Weaker than `@pure` and describes a function that
does not change any hidden state, but may depend on hidden state.
Such a function may be dead code eliminated, and may be reordered
or combined with other `@noSideEffects` or `@pure` calls, but not
Copy link
Contributor

Choose a reason for hiding this comment

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

If we do not know when the hidden state may be mutated this may make reordering difficult.
Perhaps we want to discuss also @synchronous and @asynchronous annotations, because I suspect that noSideEffects functions are only allowed to operate on state that is changed synchronously - by other operations invoked explicitly from the dataplane.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am happy to discuss the meaning of synchronous and asynchronous annotations, as it isn't clear to me yet what they are intended to mean, at least not in a precise covers-all-cases kind of way.

I am assuming that a compiler can never really use one of these proposed annotations in isolation and assume it can do safe optimizations based only upon that, but most know globally about the effects of all possible methods on an extern.

Does that change the meaning of the proposed @nosideeffects annotation, though? Or is it a caution to compiler optimization pass writers, who are hopefully already a cautious group of folks? :-)

accessed state is limited to state associated directly with the extern
object instance on which the method is called. When applied to an extern
function, the accessible state is restricted to state associated directly
with this extern function. Method and function calls so annotated will
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect that for extern functions you imply that they operate on a "singleton" instance of an implicit built-in extern?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is the meaning I was trying to convey by the sentence "When applied to an extern function, the accessible state is restricted to state associated directly with this extern function."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is essentially what I was trying to say here. Not clear if introducing the concept of an implicit singleton makes it clearer or less clear though.

being processed by the P4_16 code that made the method/function call. An
annotation of `@packetState` implies an annotation of `@localState`.

- `@localIndexedState` - Similar to `@localState`, except it is even
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that all these would benefit from at least an example.
I am not sure that the compiler can really take much advantage of the localIndexedState if nothing more is said about the partitioning index and how it is used as a function parameter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Before writing examples in the spec, I think it is better to consider leaving localState, localIndexedState, and packetState out of the spec, if they have no known use cases for enabling compiler optimizations. I do think the ideas behind them are useful for documentation purposes, whether they are annotations in the language or not.

never read, nor write, global state of the architecture, nor the state
of any other extern object instance or function.

- `@packetState` - Similar to `@localState`, except it is even more
Copy link
Contributor

Choose a reason for hiding this comment

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

I wrote this comment on the compiler PR:
It looks to me like we may want this annotation to be on the extern itself rather than on each method.
Does it ever make sense to have only some methods of an extern be @packetState?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not know if it ever makes sense to have only some methods of an extern be @packetState and not others.

I can say that in v1model and PSA architectures, the only ones that exist where @packetState appears correct are:

  • some of the extern functions
  • for PSA's Checksum, Digest, and InternetChecksum externs, where it does apply to all methods of the extern, not only some of them.

Details are in this article, where you can look for yes's in the @packetState column of the two tables: https://github.com/jafingerhut/p4-guide/tree/master/extern-annotations

Again, before we try to make localState, localIndexedState, and packetState more precise in the spec, better to decide whether we should leave them out of the spec, at least for now, until and unless they are deemed useful for compiler optimization passes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't at the moment think of an extern where we might want different annotations on different methods, but I'm not sure moving it to the extern (requiring all methods of an extern to be the same) really buys us anything (and it makes the implementation a little more cumbersome.)

@ChrisDodd
Copy link
Contributor Author

One thought from WG discussion -- would users want to put these annotations on any of their functions? Since the compiler always has the function bodies available for analysis, it does not need them, but perhaps the user might signal their intentions with the annotations? Then if the compiler determines they are not met (ie finds a side effect in a @nosideeffects function) it could issue a diagnostic.

@jafingerhut
Copy link
Collaborator

@pure and @noSideEffects annotations were added to the spec with PR #835. This PR is open in case we later want to add any of the other annotations mentioned in this PR, e.g. @localState @packetState @localIndexedState. It is not clear yet whether these other annotations should be defined differently, or at least named differently, and whether they provide enough value to a compiler developer for any kinds of optimizations. At the moment, they appear to be at best a way to document to an interested person what the restrictions on the extern behavior are.

@jnfoster
Copy link
Collaborator

jnfoster commented Feb 7, 2022

We decided to close this issue at the 2/7/22 LDWG as @pure and @noSideEffects have already been adopted in the spec.

@jnfoster jnfoster closed this Feb 7, 2022
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.

4 participants