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

Generalized array support #5115

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

ChrisDodd
Copy link
Contributor

First change relaxes frontend checks to allow arrays in general as per p4lang/p4-spec#1320. Non-header arrays only support indexing, not any other header stack operations

Second change adds support for C-style array declarators, with the array size after the name instead of before it.

Relax the requirement that IR::Type_Stack element types are headers,
which allows using arrays of any type, though non-headers cannot
support push/pop/last/next stuff which looks at header valid bits.

Signed-off-by: Chris Dodd <[email protected]>
- this allows declaring arrays as in C (eg, `bit<8> data[16];`) rather
  than in the java style previously required (`bit<8> [16] data;`)

Signed-off-by: Chris Dodd <[email protected]>
@jafingerhut
Copy link
Contributor

Nice! The test cases look interesting. As usual (unforunately), I haven't attempted to review the C++ implementation code.

What is the behavior if you attempt to use an index expression that is not a compile-time known value?

I could imagine trying to allow that if the index expression type was bit<W>, and the array had size at least 2^W, since you can pretty easily statically guarantee that the index is in range in that case. Anything fancier than that and it starts to go down the rabbit hole of what can be proved about the range of values at compile time.

@ChrisDodd
Copy link
Contributor Author

What is the behavior if you attempt to use an index expression that is not a compile-time known value?

I could imagine trying to allow that if the index expression type was bit<W>, and the array had size at least 2^W, since you can pretty easily statically guarantee that the index is in range in that case. Anything fancier than that and it starts to go down the rabbit hole of what can be proved about the range of values at compile time.

The frontend/midend won't do anything about it, but I would imaging that many targets would be unable to support non-constant indexes and the backend would reject it. It is not clear what a target that could support non-constant indexes could/should do about out of range indexes. In all of this, an array is the same as a header stack. This really just makes header stacks a special case of arrays -- they support some additional functions that arrays of non-header/header union types do not.

Comment on lines +4 to +10
header data_t {
bit<32> f1;
bit<32> f2;
bit<16> h1;
bit<16> h2;
bit<8>[16] arr;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add positive and/or error test cases similar to the following to demonstrate what is expected when the array type is an array type or contains array types?

struct S {
    bit<16> f1;
    bit<8>[16] nested_arr;
    bit<16> f2;
}

header data_t {
    bit<32>     f1;
    bit<32>     f2;
    bit<16>     h1;
    bit<16>     h2;
    S[16]  arr;
}
...
    hdr.data.arr[x].nested_arr[y] = ...;
    ... = hdr.data.arr[z].nested_arr[w];
...

and (if the grammar allows it (I did not look very closely at changes to p4parser.ypp)):

header data_t {
    bit<32>     f1;
    bit<32>     f2;
    bit<16>     h1;
    bit<16>     h2;
    bit<8>[16][16]  arr;
}
...
    hdr.data.arr[x][y] = ...;
    ... = hdr.data.arr[z][w];
...

@fruffy fruffy added the p4-spec Topics related to the P4 specification (https://github.com/p4lang/p4-spec/). label Jan 31, 2025
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