Support for list types and literals

[mihaibudiu]

Signed-off-by: Mihai Budiu [email protected]

This is a prototype implementation supporting vector types and literals, that will be discussed in the P4 design working group.
Vector literals are inspired by Rust and specify the vector type:
[bit<32>; 2, 3, 4] is a vector literal having type Vector<bit<32>>

[apinski-cavium]

I am trying to understand why is vector types just not a special case of a tuple type.
Or is it because a vector type has no fixed length when passed while a tuple is fixed in the # of elements?

[mihaibudiu]

Yes, it's similar to a stack, but does not constrain the elements to be headers, and does not have a fixed size.
A tuple has a fixed number of fields, and they can be of different types.
All elements of a vector must have the same type.
We could have used tuples for Andy's example, but the typechecking would not be generic, rather it would be done by the backend.

[mihaibudiu]

We've never had a stack literal, we can use this syntax for that purpose as well, allowing a vector literal to be used as an initializer for a stack.

[apinski-cavium]

A few questions about constraints and operations on this type.
I assume since the type is all the same, then vector[nonconst] would be valid as you know the type.
Also will there be a vector.length?
Is it valid to pass directionalless, in, out or inout?
If allowed out (or inout), when is the length assigned after the call I assume.
Is vector[n] a lvalue or is the vector always constant after assigned to? Is the size constant after it has been assigned, that is can you add an element after it was assigned, delete one?
That is it is similar to c++ vector type?

In c++ you can do now std::vector{a, b, c}. I feel like we already have this syntax with (type){} so why do we need a new syntax like you added.
Why not (vector<bit<32>>){1, 2. 3} instead of what you added which seems out of place compared to the rest of the syntax?

[mihaibudiu]

In the future we could add more operations to the vector type, but at this point there aren't any. The only thing you can do is to declare a literal, which is what Andy asked for.
If we really want a dynamically mutable vector probably we should declare an extern type with methods. But today externs do not have associated literals, and for vectors literals seem most useful. We could compromise and have both an extern type and an associated literal - making this an "intern" extern.
We could use the C++ syntax for literals, but that's already overloaded for structs and tuples.
This is all in scope for the discussion next Monday. This PR is just a prototype showing that it can be done.

[mihaibudiu]

A good design would leave open the possibility of extending the design in many of these directions.

[jfingerh]

I tried to create modified versions of the ExactMap and TernaryMap externs that at least partially motivated these changes. I have attached them in a ZIP file to this issue, but you can also find them at this link: https://github.com/jafingerhut/p4-guide/tree/master/issues/p4c-pr-3520

My first try can be seen in file https://github.com/jafingerhut/p4-guide/blob/master/issues/p4c-pr-3520/use-vector-for-exactmap-try1.p4
With these proposed changes, that program gives the following errors from running p4test use-vector-for-exactmap-try1.p4: https://github.com/jafingerhut/p4-guide/blob/master/issues/p4c-pr-3520/use-vector-for-exactmap-try1.error-output.txt

Is that expected?

I was able to create a different version of an ExactMap extern here, but it required a third type parameter, one that the P4 developer is expected to make a struct that contains fields that match the types K and V: https://github.com/jafingerhut/p4-guide/blob/master/issues/p4c-pr-3520/use-vector-for-exactmap-try2.p4 That program gave no errors from p4test.

I did a similar thing for the TernaryMap extern, with similar errors for my first try, and a similar workaround that gives no errors, but with an additional imposition on the P4 developers instantiating such an extern.

Is there a way with these changes alone to create an ExactMap extern definition that has only the type parameters K and V, and allows one to express somehow within the definition of the ExactMap extern that you want the const_entries constructor parameter to have a type that is a Vector of K, V pairs (whether those pairs are tuple, list, struct, etc. I do not have any preference)?

p4c-pr-3520-exactmap-ternarymap-tries.zip

[mihaibudiu]

For some reason the compiler infers the type of the tuples as being a struct instead of a tuple.
I will work to improve this. Your first example should work in the end.

[apinski-cavium]

I have a small nit if we go down the vector route, no other builtin type uses Camel Case so I would have expected the keyword to be vector rather than Vector.

[mihaibudiu]

If the desire is to support a runtime vector, with insertion, deletion, membership, etc operations, the right way to do it is to actually define a standard library extern type with the suitable API. Vector<T> is the type of an expression that evaluates to a vector, that may be used for example to initialize such an extern. So the two notions are complementary.

[mihaibudiu]

I have amended the PR to support the syntax suggested by the LDWG for vector literals. Here is an example:

extern E<T, S> {
    E(Vector<tuple<T, S>> data);
    void run();
}

control c() {
    E<bit<32>, bit<16>>((Vector<tuple<bit<32>, bit<16>>>){{2, 3}, {4, 5}}) e;
    apply {
        e.run();
    }
}

[jnfoster]

Sent @mbudiu-vmw a private note, but I'd like us to strongly consider rationalizing the terminology for the following concepts, to align with the standard usage:

• tuples: ordered collections of heterogeneous elements
• lists: ordered collections of homogeneous elements

I don't think P4 needs vectors (i.e., extensible, ordered collections of homogeneous elements) just yet.

I haven't worked through all the corner cases yet, so I may well be missing something major. But this may be as simple as a few tweaks to the spec, and renaming vector to list here. We don't currently have a list type. In fact, we say that list expressions generate tuple types (as do tuple expressions). So I think with a tiny bit of smoothing out we can probably patch this up with minimal impact.

[jfingerh]

@jnfoster Not trying to stir the pot, but if naming the new thing list instead of Vector goes through (sounds reasonable to me), then would it be preferable to change the spec so that everywhere it currently says "list expression" it will instead say "tuple expression"? It turns out that there are only 14 occurrences in the latest spec of the phrase "list expression". There are more occurrences of just "list", many of which should not be changed to "tuple", but many that probably should, if we went with that.

[mihaibudiu]

Yes, that will be a problem, since the existing list type has actually no connection with these new list expressions and the second kind of list type.

[jnfoster]

But list types only appear in the spec and internally. There is no syntax for them. So this doesn't seem as bad as it could be...

[jafingerhut]

I went through all occurrences of the word 'list' in the current spec that were not "list expression", and nearly all of them occurred in phrases like "actions list", "parameter list", "argument list", etc., i.e. they were just the normal English use of the word list, not referring to a list type.

[jafingerhut]

I am attaching 4 more test programs in a zip file. With the changes proposed after 4 commits on this PR, vector5.p4 and vector6.p4 compile without errors, as I would hope. I would ask that they be considered as additional test programs to be checked in for this feature.

The programs vector3.p4 gives a CompilerBug, and vector4.p4 gives an odd looking error message that it cannot match type signatures that look identical to me in the error message. I am not sure if the use of int is not expected to be a type that is supported in those examples.

more-vector-test-programs.zip

[mihaibudiu]

@jafingerhut all the tests that you have submitted now pass. There was a bug which would have caused trouble with other nested types, not just the new list type. I have renamed Vector to list.

[jfingerh]

Thanks again! The test cases and their expected outputs all look correct to me. As usual, I'm not attempting to review the implementation code within the compiler.

[jfingerh]

LGTM. As usual, I am reviewing the test programs and their expected outputs only, not the compiler implementation code.

[jafingerhut]

Minor comment: It might be nice to rename the test programs to have 'list' in their file names, rather than 'vector', if we name the new data type 'list'.

[jafingerhut]

Another minor comment: The names of non-terminal symbols in the grammar should also probably be renamed to have 'list' in their names rather than 'vector', if we go with 'list'.

[mihaibudiu]

The list names are taken and if we change them we may break backends

[jnfoster]

@mbudiu-vmw do you mean the names of the IR nodes? That seems different than the names of non-terminals in the grammar.

[jfingerh]

I have updated and rebuilt, and the program in the zip file I named p4c-list-issue1.zip now compiles without errors or compiler bugs, the one on this comment: #3520 (comment) (you can click to follow the link).

The zip file named p4c-list-issues2.zip still has a compiler bug when I try to compile the program within it. It is on this comment: #3520 (comment)

[mihaibudiu]

This will wait for #3686 to be merged first. Hopefully this will fix the problems discovered here.

[jfingerh]

Now that #3686 has been merged, if you get a chance to rebase this on latest p4c/main code, I would be happy to test it out on my latest example programs that have been giving me errors.

[mihaibudiu]

@jafingerhut I cannot locate the file p4c-list-issue2.zip. Where is it?

[jfingerh]

Sorry, I attempted to upload p4c-list-issue2.zip to a previous comment, but apparently it did not complete. Trying again to attach it to this comment.
p4c-list-issue2.zip

[mihaibudiu]

@jafingerhut your test still fails, let me take a look at that. It may be an unrelated bug.

[mihaibudiu]

@jafingerhut this should pass your tests as well. If the tests pass I plan to merge this PR.

[jfingerh]

Confirmed that one of the last few commits fixed the p4c-list-issue2.zip program CompilerBug. Thanks!