P4_14 -> P4_16 conversion for externs #162
Conversation
|
Not super familiar with this part of the code...can you explain what exactly is going on here/what changes are going in at a high level? |
|
P4 v1.1 introduced a specific syntax for extern blocks. |
|
I see, thanks Mihai. |
|
This is all code associated with handle extern objects. The p4-14/typecheck.cpp is p4-14 typechecking, which replaces NamedRef (simple name expressions in the code) with GlobalRef and AttributeRef objects referring to global instances of things (counter and meters as well as externs) and attributes of externs; backends will need these to deal with these objects properly) Most of the changes are in the P4-14 to P4-16 conversion, where global extern instances from P4-14 are converted to extern instance declarations in the control flows containing the tables and actions that use the extern instance. I mostly assigned Pierce in case any of this impacts any of the architecture stuff you're currently working on -- when we move this path to use the standard switch architecture rather than the current v1model.p4, much of this code (the from1.0 stuff) will be impacted. |
|
Thanks Chris. |
| @@ -31,6 +31,13 @@ class TypeCheck::Pass1 : public Transform { | |||
| if (auto af = findContext<IR::ActionFunction>()) | |||
| if (auto arg = af->arg(ref->name)) | |||
| return arg; | |||
| if (auto bbox = findContext<IR::Declaration_Instance>()) { | |||
There was a problem hiding this comment.
There's something I don't understand:
- p4-14 is compiled just for the v1model.p4
- the list of externs in v1model.p4 is fixed
So I don't understand how new extern declarations can appear in P4-14 programs. P4-14 does not support multiple architectures, and our front-end which parses p4-14 is hardwired to assume v1model.p4. So how can one write a p4 program that uses the extern syntax that has attributes if your target does not support any such externs?
| return ref; } | ||
| if (auto hdr = findContext<IR::Type_StructLike>()) { | ||
| if (auto field = hdr->getField(ref->name)) { | ||
| /* FIXME -- Should this be converted to something else? */ |
There was a problem hiding this comment.
There are lots of FIXME here...
Do we have any input p4 programs that will exercise this code?
There was a problem hiding this comment.
We have a number of Barefoot internal testcases (using stateful ALU externs) that exercise this code. Unfortunately nothing on the open-source side (yet?)
|
|
||
| if (extrn) { | ||
| auto parent = findContext<IR::ActionFunction>(); | ||
| BUG_CHECK(parent != nullptr, "%1%: Extern call not within action", primitive); |
There was a problem hiding this comment.
Should this be ::error instead of BUG?
There was a problem hiding this comment.
P4-14 syntactically disallow statements (primitive calls in this case) anywhere other than in action bodies, so it should not occur.
| validate{ BUG_CHECK(type->is<Type_Name>() || | ||
| type->is<Type_Specialized>(), | ||
| type->is<Type_Specialized>() || | ||
| type->is<Type_Extern>(), // P4_14 only? |
There was a problem hiding this comment.
For P4-16 this can be a Type_Name that refers to a Type_Extern, but not the Type_Extern itself. I don't see any call to a Declaration_Instance constructor in the new code which can use a Type_Extern. Why is this needed?
There was a problem hiding this comment.
The P4_14 parser produces DeclarationInstances that refer to Type_Externs.
| @@ -22,7 +22,7 @@ header ipv4_t { | |||
| bit<16> hdrChecksum; | |||
| bit<32> srcAddr; | |||
| bit<32> dstAddr; | |||
| @length(ihl * 4) | |||
| @length(ihl * 4w4) | |||
There was a problem hiding this comment.
Can you please commit some tests which exercise these paths as well?
This is incomplete as it doesn't convert P4_14 style extern attributes into P4_16 style arguments to the extern constructor. Since we don't dump the attributes when producing P4_16 code, it results in the attributes simply being lost, which is bad.