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.

Sign up for GitHub

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
Closed

standard annotations on extern functions/methods #802

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe45f56
standard annotations on extern functions/methods
Dec 30, 2019
b1badd1
typo fixes
Jan 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions p4-16/spec/P4-16-spec.mdk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7200,6 +7200,63 @@ The `@match` annotation, described in Section [#sec-select], is used
to specify a `match_kind` value other than the default `match_kind` of
`exact` for a field of a `value_set`.

### Extern function/method annotations { #sec-extern-annotations }

Various annotations may appear on extern function and method declarations
to describe limitations on the behavior and interactions of those
functions. By default extern functions might have any effect on the
environment of the P4 program and might interact in non-trivial ways.
Since externs are architecture-specific and their behavior is known to
the architecture definition, these annotations are not strictly necessary
(an implementation can have knowledge of how externs interact based
on their names built into it), but these annotations provide a uniform
way of describing certain well-defined interactions (or their absence),
allowing architecture-indepedent analysis of P4 programs.

- `@pure` - Describes a function that depends solely on its `in` argument
values and has no effect other than its `out` argument values and return
value. No hidden state is recorded between calls, and its value does
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 multiple calls
with identical arguments may be combined into a single call. `@pure`
functions may also be reordered with respect to other computations
that are not data dependent.

- `@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? :-)

with other function calls that may have side effects that affect
the function.

- `@localState` - When applied to an extern method, the extern method
implementation might read and/or write internal state, but all such
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.

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.)

restricted in the state that can be accessed. The internal state of the
extern object instance or extern function is partitioned by packets being
processed in the device. The extern method/function can only access the
part of its internal state that is associated with the current packet
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.

more restricted in the state that can be accessed. The internal state
of the extern object instance or extern function is partitioned into an
array of independently accessible sub-states, each with its own index or
associated table entry. The extern method/function call can only access
the internal state associated with that one index, or one table entry,
and no others. An annotation of `@localIndexedState` implies an annotation
of `@localState`.

### Deprecated annotation { #sec-deprecated-anno }

The `deprecated` annotation has a required string argument that is a
Expand Down Expand Up @@ -7329,6 +7386,11 @@ The following table shows all P4 reserved annotations.
| `optional` | parameter is optional | [#sec-optional-parameter-annotations] |
| `tableonly` | action cannot be a defalt_action | [#sec-table-action-anno] |
| `deprecated` | Construct has been deprecated | [#sec-deprecated-anno] |
| `pure` | pure function | [#sec-extern-annotations] |
| `noSideEffects` | function with no side effects | [#sec-extern-annotations] |
| `localState` | limited hidden state | [#sec-extern-annotations] |
| `packetState` | limited hidden state | [#sec-extern-annotations] |
| `localIndexedState`| limited hidden state | [#sec-extern-annotations] |
|---------------|----------------------------------------------------|--------------------------|

# Appendix: P4 core library { #sec-p4-core-lib }
Expand Down