From ae182dba9f4f33df00700c6c3db918775c99236b Mon Sep 17 00:00:00 2001 From: Chris Dodd Date: Wed, 29 Jan 2025 00:11:52 +0000 Subject: [PATCH 1/4] Allow arrays of non-header types in frontend 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 --- frontends/p4/typeChecking/typeCheckExpr.cpp | 7 +- frontends/p4/typeChecking/typeCheckTypes.cpp | 8 ++- frontends/parsers/p4/p4parser.ypp | 3 +- testdata/p4_16_errors/stack_e.p4 | 3 +- testdata/p4_16_errors_outputs/stack_e.p4 | 1 + .../p4_16_errors_outputs/stack_e.p4-stderr | 6 +- testdata/p4_16_samples/array1.p4 | 71 +++++++++++++++++++ .../p4_16_samples_outputs/array1-first.p4 | 61 ++++++++++++++++ .../p4_16_samples_outputs/array1-frontend.p4 | 61 ++++++++++++++++ .../p4_16_samples_outputs/array1-midend.p4 | 70 ++++++++++++++++++ testdata/p4_16_samples_outputs/array1.p4 | 61 ++++++++++++++++ .../p4_16_samples_outputs/array1.p4-stderr | 0 .../array1.p4.entries.txtpb | 3 + .../array1.p4.p4info.txtpb | 6 ++ 14 files changed, 353 insertions(+), 8 deletions(-) create mode 100644 testdata/p4_16_samples/array1.p4 create mode 100644 testdata/p4_16_samples_outputs/array1-first.p4 create mode 100644 testdata/p4_16_samples_outputs/array1-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/array1-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/array1.p4 create mode 100644 testdata/p4_16_samples_outputs/array1.p4-stderr create mode 100644 testdata/p4_16_samples_outputs/array1.p4.entries.txtpb create mode 100644 testdata/p4_16_samples_outputs/array1.p4.p4info.txtpb diff --git a/frontends/p4/typeChecking/typeCheckExpr.cpp b/frontends/p4/typeChecking/typeCheckExpr.cpp index 851a5de79bb..b711c581a0a 100644 --- a/frontends/p4/typeChecking/typeCheckExpr.cpp +++ b/frontends/p4/typeChecking/typeCheckExpr.cpp @@ -1706,7 +1706,12 @@ const IR::Node *TypeInferenceBase::postorder(const IR::Member *expression) { if (auto *stack = type->to()) { auto parser = findContext(); - if (member == IR::Type_Stack::next || member == IR::Type_Stack::last) { + auto eltype = stack->elementType; + if (!eltype->is() && !eltype->is() /* && + !eltype->is() */) { + typeError("%1%: '%2%' can only be used on header stacks", expression, member); + return expression; + } else if (member == IR::Type_Stack::next || member == IR::Type_Stack::last) { if (parser == nullptr) { typeError("%1%: 'last', and 'next' for stacks can only be used in a parser", expression); diff --git a/frontends/p4/typeChecking/typeCheckTypes.cpp b/frontends/p4/typeChecking/typeCheckTypes.cpp index ab9ee9dc50b..9a9cbb7415e 100644 --- a/frontends/p4/typeChecking/typeCheckTypes.cpp +++ b/frontends/p4/typeChecking/typeCheckTypes.cpp @@ -382,9 +382,11 @@ const IR::Node *TypeInferenceBase::postorder(const IR::Type_Stack *type) { auto etype = canon->to()->elementType; if (etype == nullptr) return type; +#if 0 if (!etype->is() && !etype->is() && !etype->is()) typeError("Header stack %1% used with non-header type %2%", type, etype->toString()); +#endif return type; } @@ -422,7 +424,11 @@ const IR::Node *TypeInferenceBase::postorder(const IR::StructField *field) { const IR::Node *TypeInferenceBase::postorder(const IR::Type_Header *type) { auto canon = setTypeType(type); auto validator = [this](const IR::Type *t) { - while (t->is()) t = getTypeType(t->to()->type); + while (1) { + if (t->is()) t = getTypeType(t->to()->type); + else if (auto *st = t->to()) t = st->elementType; + else break; + } return t->is() || t->is() || (t->is() && onlyBitsOrBitStructs(t)) || t->is() || t->is() || t->is() || diff --git a/frontends/parsers/p4/p4parser.ypp b/frontends/parsers/p4/p4parser.ypp index dd69f9b9d8a..c95f82e0f95 100644 --- a/frontends/parsers/p4/p4parser.ypp +++ b/frontends/parsers/p4/p4parser.ypp @@ -1075,8 +1075,7 @@ tupleType ; headerStackType - : typeName "[" expression "]" { $$ = new IR::Type_Stack(@1+@4, $1, $3); } - | specializedType "[" expression "]" { $$ = new IR::Type_Stack(@1+@4, $1, $3); } + : typeRef "[" expression "]" { $$ = new IR::Type_Stack(@1+@4, $1, $3); } ; specializedType diff --git a/testdata/p4_16_errors/stack_e.p4 b/testdata/p4_16_errors/stack_e.p4 index 640126a02d0..e24df9d7503 100644 --- a/testdata/p4_16_errors/stack_e.p4 +++ b/testdata/p4_16_errors/stack_e.p4 @@ -20,11 +20,12 @@ control p() { apply { h[5] stack; - s[5] stack1; // non-header illegal in header stack + s[5] stack1; // 'normal' array can't use stack operations // out of range indexes h b = stack[1231092310293]; h c = stack[-2]; h d = stack[6]; + s e = stack1.next; } } diff --git a/testdata/p4_16_errors_outputs/stack_e.p4 b/testdata/p4_16_errors_outputs/stack_e.p4 index 010dfd5dcd1..bf48df51e19 100644 --- a/testdata/p4_16_errors_outputs/stack_e.p4 +++ b/testdata/p4_16_errors_outputs/stack_e.p4 @@ -11,6 +11,7 @@ control p() { h b = stack[1231092310293]; h c = stack[-2]; h d = stack[6]; + s e = stack1.next; } } diff --git a/testdata/p4_16_errors_outputs/stack_e.p4-stderr b/testdata/p4_16_errors_outputs/stack_e.p4-stderr index 26b3629ed0d..0424602b7b4 100644 --- a/testdata/p4_16_errors_outputs/stack_e.p4-stderr +++ b/testdata/p4_16_errors_outputs/stack_e.p4-stderr @@ -1,6 +1,6 @@ -stack_e.p4(23): [--Werror=type-error] error: Header stack struct s[5] used with non-header type struct s - s[5] stack1; // non-header illegal in header stack - ^^^^ stack_e.p4(26): [--Werror=overlimit] error: 1231092310293: Value too large for int h b = stack[1231092310293]; ^^^^^^^^^^^^^ +stack_e.p4(29): [--Werror=type-error] error: stack1.next: 'next' can only be used on header stacks + s e = stack1.next; + ^^^^^^^^^^^ diff --git a/testdata/p4_16_samples/array1.p4 b/testdata/p4_16_samples/array1.p4 new file mode 100644 index 00000000000..017aec6cc54 --- /dev/null +++ b/testdata/p4_16_samples/array1.p4 @@ -0,0 +1,71 @@ +#include +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8>[16] arr; +} + +struct metadata {} + +struct headers { + data_t data; +} + +parser p(packet_in b, + out headers hdr, + inout metadata meta, + inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, + inout metadata meta, + inout standard_metadata_t stdmeta) { + apply { + hdr.data.arr[0] = hdr.data.f1[0+:8]; + hdr.data.arr[1] = hdr.data.f1[8+:8]; + hdr.data.arr[2] = hdr.data.f1[16+:8]; + hdr.data.arr[3] = hdr.data.f1[24+:8]; + hdr.data.arr[4] = hdr.data.f2[0+:8]; + hdr.data.arr[5] = hdr.data.f2[8+:8]; + hdr.data.arr[6] = hdr.data.f2[16+:8]; + hdr.data.arr[7] = hdr.data.f2[24+:8]; + } +} + +control egress(inout headers hdr, + inout metadata meta, + inout standard_metadata_t stdmeta) { + apply {} +} + +control vc(inout headers hdr, + inout metadata meta) { + apply {} +} + +control uc(inout headers hdr, + inout metadata meta) { + apply {} +} + +control deparser(packet_out packet, + in headers hdr) { + apply { + packet.emit(hdr); + } +} + +V1Switch(p(), + vc(), + ingress(), + egress(), + uc(), + deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array1-first.p4 b/testdata/p4_16_samples_outputs/array1-first.p4 new file mode 100644 index 00000000000..04c776aea2b --- /dev/null +++ b/testdata/p4_16_samples_outputs/array1-first.p4 @@ -0,0 +1,61 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8>[16] arr; +} + +struct metadata { +} + +struct headers { + data_t data; +} + +parser p(packet_in b, out headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + hdr.data.arr[0] = hdr.data.f1[7:0]; + hdr.data.arr[1] = hdr.data.f1[15:8]; + hdr.data.arr[2] = hdr.data.f1[23:16]; + hdr.data.arr[3] = hdr.data.f1[31:24]; + hdr.data.arr[4] = hdr.data.f2[7:0]; + hdr.data.arr[5] = hdr.data.f2[15:8]; + hdr.data.arr[6] = hdr.data.f2[23:16]; + hdr.data.arr[7] = hdr.data.f2[31:24]; + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control vc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control uc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control deparser(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr); + } +} + +V1Switch(p(), vc(), ingress(), egress(), uc(), deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array1-frontend.p4 b/testdata/p4_16_samples_outputs/array1-frontend.p4 new file mode 100644 index 00000000000..04c776aea2b --- /dev/null +++ b/testdata/p4_16_samples_outputs/array1-frontend.p4 @@ -0,0 +1,61 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8>[16] arr; +} + +struct metadata { +} + +struct headers { + data_t data; +} + +parser p(packet_in b, out headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + hdr.data.arr[0] = hdr.data.f1[7:0]; + hdr.data.arr[1] = hdr.data.f1[15:8]; + hdr.data.arr[2] = hdr.data.f1[23:16]; + hdr.data.arr[3] = hdr.data.f1[31:24]; + hdr.data.arr[4] = hdr.data.f2[7:0]; + hdr.data.arr[5] = hdr.data.f2[15:8]; + hdr.data.arr[6] = hdr.data.f2[23:16]; + hdr.data.arr[7] = hdr.data.f2[31:24]; + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control vc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control uc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control deparser(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr); + } +} + +V1Switch(p(), vc(), ingress(), egress(), uc(), deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array1-midend.p4 b/testdata/p4_16_samples_outputs/array1-midend.p4 new file mode 100644 index 00000000000..395f631c152 --- /dev/null +++ b/testdata/p4_16_samples_outputs/array1-midend.p4 @@ -0,0 +1,70 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8>[16] arr; +} + +struct metadata { +} + +struct headers { + data_t data; +} + +parser p(packet_in b, out headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + @hidden action array1l32() { + hdr.data.arr[0] = hdr.data.f1[7:0]; + hdr.data.arr[1] = hdr.data.f1[15:8]; + hdr.data.arr[2] = hdr.data.f1[23:16]; + hdr.data.arr[3] = hdr.data.f1[31:24]; + hdr.data.arr[4] = hdr.data.f2[7:0]; + hdr.data.arr[5] = hdr.data.f2[15:8]; + hdr.data.arr[6] = hdr.data.f2[23:16]; + hdr.data.arr[7] = hdr.data.f2[31:24]; + } + @hidden table tbl_array1l32 { + actions = { + array1l32(); + } + const default_action = array1l32(); + } + apply { + tbl_array1l32.apply(); + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control vc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control uc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control deparser(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr.data); + } +} + +V1Switch(p(), vc(), ingress(), egress(), uc(), deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array1.p4 b/testdata/p4_16_samples_outputs/array1.p4 new file mode 100644 index 00000000000..e0fe8c3eada --- /dev/null +++ b/testdata/p4_16_samples_outputs/array1.p4 @@ -0,0 +1,61 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8>[16] arr; +} + +struct metadata { +} + +struct headers { + data_t data; +} + +parser p(packet_in b, out headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + hdr.data.arr[0] = hdr.data.f1[0+:8]; + hdr.data.arr[1] = hdr.data.f1[8+:8]; + hdr.data.arr[2] = hdr.data.f1[16+:8]; + hdr.data.arr[3] = hdr.data.f1[24+:8]; + hdr.data.arr[4] = hdr.data.f2[0+:8]; + hdr.data.arr[5] = hdr.data.f2[8+:8]; + hdr.data.arr[6] = hdr.data.f2[16+:8]; + hdr.data.arr[7] = hdr.data.f2[24+:8]; + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control vc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control uc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control deparser(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr); + } +} + +V1Switch(p(), vc(), ingress(), egress(), uc(), deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array1.p4-stderr b/testdata/p4_16_samples_outputs/array1.p4-stderr new file mode 100644 index 00000000000..e69de29bb2d diff --git a/testdata/p4_16_samples_outputs/array1.p4.entries.txtpb b/testdata/p4_16_samples_outputs/array1.p4.entries.txtpb new file mode 100644 index 00000000000..5cb9652623a --- /dev/null +++ b/testdata/p4_16_samples_outputs/array1.p4.entries.txtpb @@ -0,0 +1,3 @@ +# proto-file: p4/v1/p4runtime.proto +# proto-message: p4.v1.WriteRequest + diff --git a/testdata/p4_16_samples_outputs/array1.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/array1.p4.p4info.txtpb new file mode 100644 index 00000000000..fdf16790b91 --- /dev/null +++ b/testdata/p4_16_samples_outputs/array1.p4.p4info.txtpb @@ -0,0 +1,6 @@ +# proto-file: p4/config/v1/p4info.proto +# proto-message: p4.config.v1.P4Info + +pkg_info { + arch: "v1model" +} From 72e4e7808b50ce346f3474824dd51939ab8678a6 Mon Sep 17 00:00:00 2001 From: Chris Dodd Date: Wed, 29 Jan 2025 06:28:24 +0000 Subject: [PATCH 2/4] Allow C-style array declarators after the name - 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 --- frontends/parsers/p4/p4parser.ypp | 64 ++++++++++++----- testdata/p4_16_samples/array2.p4 | 71 +++++++++++++++++++ .../p4_16_samples_outputs/array2-first.p4 | 61 ++++++++++++++++ .../p4_16_samples_outputs/array2-frontend.p4 | 61 ++++++++++++++++ .../p4_16_samples_outputs/array2-midend.p4 | 70 ++++++++++++++++++ testdata/p4_16_samples_outputs/array2.p4 | 61 ++++++++++++++++ .../p4_16_samples_outputs/array2.p4-stderr | 0 .../array2.p4.entries.txtpb | 3 + .../array2.p4.p4info.txtpb | 6 ++ 9 files changed, 379 insertions(+), 18 deletions(-) create mode 100644 testdata/p4_16_samples/array2.p4 create mode 100644 testdata/p4_16_samples_outputs/array2-first.p4 create mode 100644 testdata/p4_16_samples_outputs/array2-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/array2-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/array2.p4 create mode 100644 testdata/p4_16_samples_outputs/array2.p4-stderr create mode 100644 testdata/p4_16_samples_outputs/array2.p4.entries.txtpb create mode 100644 testdata/p4_16_samples_outputs/array2.p4.p4info.txtpb diff --git a/frontends/parsers/p4/p4parser.ypp b/frontends/parsers/p4/p4parser.ypp index c95f82e0f95..6ce23efad4e 100644 --- a/frontends/parsers/p4/p4parser.ypp +++ b/frontends/parsers/p4/p4parser.ypp @@ -91,6 +91,15 @@ inline std::ostream& operator<<(std::ostream& out, const OptionalConst& oc) { // forbidden in C++. We avoid the problem using a typedef. typedef const IR::Type ConstType; +struct NameTypePair { + const IR::ID *name; + const IR::Type *type; +}; + +inline std::ostream &operator<<(std::ostream &out, const NameTypePair &nt) { + return out << nt.name << ":" << nt.type; +} + } // namespace P4 #ifndef YYDEBUG @@ -251,6 +260,7 @@ using namespace P4; %type name dot_name nonTypeName nonTableKwName annotationName +%type declarator %type direction %type prefixedNonTypeName prefixedType %type*> typeParameterList @@ -262,6 +272,7 @@ using namespace P4; nonBraceExpression forCollectionExpr %type baseType typeOrVoid specializedType headerStackType typeRef tupleType typeArg realTypeArg namedType p4listType + derivedTypeDeclarationAsType %type typeName %type argument %type*> argumentList nonEmptyArgList @@ -745,8 +756,10 @@ nonEmptyParameterList ; parameter - : optAnnotations direction typeRef name { $$ = new IR::Parameter(@4, *$4, *$1, $2, $3, nullptr); } - | optAnnotations direction typeRef name "=" expression { $$ = new IR::Parameter(@4, *$4, *$1, $2, $3, $6); } + : optAnnotations direction typeRef declarator { + $$ = new IR::Parameter(@4, *$4.name, *$1, $2, $4.type, nullptr); } + | optAnnotations direction typeRef declarator "=" expression { + $$ = new IR::Parameter(@4, *$4.name, *$1, $2, $4.type, $6); } ; direction @@ -1168,6 +1181,12 @@ derivedTypeDeclaration | enumDeclaration { $$ = $1; } ; +// workaround for bison's broken type variant handling that needs an explicit conversion +// to the base class +derivedTypeDeclarationAsType + : derivedTypeDeclaration { $$ = static_cast($1); } + ; + headerTypeDeclaration : optAnnotations HEADER name { driver.structure->pushContainerType(*$3, true); } optTypeParameters { driver.structure->markAsTemplate(*$3); @@ -1198,7 +1217,7 @@ structFieldList ; structField - : optAnnotations typeRef name ";" { $$ = new IR::StructField(@3, *$3, *$1, $2); } + : optAnnotations typeRef declarator ";" { $$ = new IR::StructField(@3, *$3.name, *$1, $3.type); } ; enumDeclaration @@ -1240,12 +1259,15 @@ identifierList ; typedefDeclaration - : optAnnotations TYPEDEF typeRef name { driver.structure->declareType(*$4); - $$ = new IR::Type_Typedef(@4, *$4, *$1, $3); } - | optAnnotations TYPEDEF derivedTypeDeclaration name { driver.structure->declareType(*$4); - $$ = new IR::Type_Typedef(@4, *$4, *$1, $3); } - | optAnnotations TYPE typeRef name { driver.structure->declareType(*$4); - $$ = new IR::Type_Newtype(@4, *$4, *$1, $3); } + : optAnnotations TYPEDEF typeRef declarator { + driver.structure->declareType(*$4.name); + $$ = new IR::Type_Typedef(@4, *$4.name, *$1, $4.type); } + | optAnnotations TYPEDEF derivedTypeDeclarationAsType declarator { + driver.structure->declareType(*$4.name); + $$ = new IR::Type_Typedef(@4, *$4.name, *$1, $4.type); } + | optAnnotations TYPE typeRef declarator { + driver.structure->declareType(*$4.name); + $$ = new IR::Type_Newtype(@4, *$4.name, *$1, $4.type); } ; /*************************** STATEMENTS *************************/ @@ -1524,18 +1546,24 @@ variableDeclaration ; variableDeclarationWithoutSemicolon - : annotations typeRef name optInitializer - { $$ = new IR::Declaration_Variable(@1+@4, *$3, *$1, $2, $4); - driver.structure->declareObject(*$3, $2->toString()); } - | typeRef name optInitializer - { $$ = new IR::Declaration_Variable(@1+@3, *$2, $1, $3); - driver.structure->declareObject(*$2, $1->toString()); } + : annotations typeRef declarator optInitializer + { $$ = new IR::Declaration_Variable(@1+@4, *$3.name, *$1, $3.type, $4); + driver.structure->declareObject(*$3.name, $3.type->toString()); } + | typeRef declarator optInitializer + { $$ = new IR::Declaration_Variable(@1+@3, *$2.name, $2.type, $3); + driver.structure->declareObject(*$2.name, $2.type->toString()); } ; constantDeclaration - : optAnnotations CONST typeRef name "=" initializer ";" - { $$ = new IR::Declaration_Constant(@4, *$4, *$1, $3, $6); - driver.structure->declareObject(*$4, $3->toString()); } + : optAnnotations CONST typeRef declarator "=" initializer ";" + { $$ = new IR::Declaration_Constant(@4, *$4.name, *$1, $4.type, $6); + driver.structure->declareObject(*$4.name, $4.type->toString()); } + ; + +declarator + : name { $$.name = $1; $$.type = $0; } + | declarator "[" expression "]" + { $$ = $1; $$.type = new IR::Type_Stack(@2+@4, $1.type, $3); } ; optInitializer diff --git a/testdata/p4_16_samples/array2.p4 b/testdata/p4_16_samples/array2.p4 new file mode 100644 index 00000000000..7d05e5e257b --- /dev/null +++ b/testdata/p4_16_samples/array2.p4 @@ -0,0 +1,71 @@ +#include +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8> arr[16]; +} + +struct metadata {} + +struct headers { + data_t data; +} + +parser p(packet_in b, + out headers hdr, + inout metadata meta, + inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, + inout metadata meta, + inout standard_metadata_t stdmeta) { + apply { + hdr.data.arr[0] = hdr.data.f1[0+:8]; + hdr.data.arr[1] = hdr.data.f1[8+:8]; + hdr.data.arr[2] = hdr.data.f1[16+:8]; + hdr.data.arr[3] = hdr.data.f1[24+:8]; + hdr.data.arr[4] = hdr.data.f2[0+:8]; + hdr.data.arr[5] = hdr.data.f2[8+:8]; + hdr.data.arr[6] = hdr.data.f2[16+:8]; + hdr.data.arr[7] = hdr.data.f2[24+:8]; + } +} + +control egress(inout headers hdr, + inout metadata meta, + inout standard_metadata_t stdmeta) { + apply {} +} + +control vc(inout headers hdr, + inout metadata meta) { + apply {} +} + +control uc(inout headers hdr, + inout metadata meta) { + apply {} +} + +control deparser(packet_out packet, + in headers hdr) { + apply { + packet.emit(hdr); + } +} + +V1Switch(p(), + vc(), + ingress(), + egress(), + uc(), + deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array2-first.p4 b/testdata/p4_16_samples_outputs/array2-first.p4 new file mode 100644 index 00000000000..04c776aea2b --- /dev/null +++ b/testdata/p4_16_samples_outputs/array2-first.p4 @@ -0,0 +1,61 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8>[16] arr; +} + +struct metadata { +} + +struct headers { + data_t data; +} + +parser p(packet_in b, out headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + hdr.data.arr[0] = hdr.data.f1[7:0]; + hdr.data.arr[1] = hdr.data.f1[15:8]; + hdr.data.arr[2] = hdr.data.f1[23:16]; + hdr.data.arr[3] = hdr.data.f1[31:24]; + hdr.data.arr[4] = hdr.data.f2[7:0]; + hdr.data.arr[5] = hdr.data.f2[15:8]; + hdr.data.arr[6] = hdr.data.f2[23:16]; + hdr.data.arr[7] = hdr.data.f2[31:24]; + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control vc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control uc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control deparser(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr); + } +} + +V1Switch(p(), vc(), ingress(), egress(), uc(), deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array2-frontend.p4 b/testdata/p4_16_samples_outputs/array2-frontend.p4 new file mode 100644 index 00000000000..04c776aea2b --- /dev/null +++ b/testdata/p4_16_samples_outputs/array2-frontend.p4 @@ -0,0 +1,61 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8>[16] arr; +} + +struct metadata { +} + +struct headers { + data_t data; +} + +parser p(packet_in b, out headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + hdr.data.arr[0] = hdr.data.f1[7:0]; + hdr.data.arr[1] = hdr.data.f1[15:8]; + hdr.data.arr[2] = hdr.data.f1[23:16]; + hdr.data.arr[3] = hdr.data.f1[31:24]; + hdr.data.arr[4] = hdr.data.f2[7:0]; + hdr.data.arr[5] = hdr.data.f2[15:8]; + hdr.data.arr[6] = hdr.data.f2[23:16]; + hdr.data.arr[7] = hdr.data.f2[31:24]; + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control vc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control uc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control deparser(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr); + } +} + +V1Switch(p(), vc(), ingress(), egress(), uc(), deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array2-midend.p4 b/testdata/p4_16_samples_outputs/array2-midend.p4 new file mode 100644 index 00000000000..045c490f837 --- /dev/null +++ b/testdata/p4_16_samples_outputs/array2-midend.p4 @@ -0,0 +1,70 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8>[16] arr; +} + +struct metadata { +} + +struct headers { + data_t data; +} + +parser p(packet_in b, out headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + @hidden action array2l32() { + hdr.data.arr[0] = hdr.data.f1[7:0]; + hdr.data.arr[1] = hdr.data.f1[15:8]; + hdr.data.arr[2] = hdr.data.f1[23:16]; + hdr.data.arr[3] = hdr.data.f1[31:24]; + hdr.data.arr[4] = hdr.data.f2[7:0]; + hdr.data.arr[5] = hdr.data.f2[15:8]; + hdr.data.arr[6] = hdr.data.f2[23:16]; + hdr.data.arr[7] = hdr.data.f2[31:24]; + } + @hidden table tbl_array2l32 { + actions = { + array2l32(); + } + const default_action = array2l32(); + } + apply { + tbl_array2l32.apply(); + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control vc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control uc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control deparser(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr.data); + } +} + +V1Switch(p(), vc(), ingress(), egress(), uc(), deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array2.p4 b/testdata/p4_16_samples_outputs/array2.p4 new file mode 100644 index 00000000000..e0fe8c3eada --- /dev/null +++ b/testdata/p4_16_samples_outputs/array2.p4 @@ -0,0 +1,61 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<16> h2; + bit<8>[16] arr; +} + +struct metadata { +} + +struct headers { + data_t data; +} + +parser p(packet_in b, out headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + state start { + b.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + hdr.data.arr[0] = hdr.data.f1[0+:8]; + hdr.data.arr[1] = hdr.data.f1[8+:8]; + hdr.data.arr[2] = hdr.data.f1[16+:8]; + hdr.data.arr[3] = hdr.data.f1[24+:8]; + hdr.data.arr[4] = hdr.data.f2[0+:8]; + hdr.data.arr[5] = hdr.data.f2[8+:8]; + hdr.data.arr[6] = hdr.data.f2[16+:8]; + hdr.data.arr[7] = hdr.data.f2[24+:8]; + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control vc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control uc(inout headers hdr, inout metadata meta) { + apply { + } +} + +control deparser(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr); + } +} + +V1Switch(p(), vc(), ingress(), egress(), uc(), deparser()) main; diff --git a/testdata/p4_16_samples_outputs/array2.p4-stderr b/testdata/p4_16_samples_outputs/array2.p4-stderr new file mode 100644 index 00000000000..e69de29bb2d diff --git a/testdata/p4_16_samples_outputs/array2.p4.entries.txtpb b/testdata/p4_16_samples_outputs/array2.p4.entries.txtpb new file mode 100644 index 00000000000..5cb9652623a --- /dev/null +++ b/testdata/p4_16_samples_outputs/array2.p4.entries.txtpb @@ -0,0 +1,3 @@ +# proto-file: p4/v1/p4runtime.proto +# proto-message: p4.v1.WriteRequest + diff --git a/testdata/p4_16_samples_outputs/array2.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/array2.p4.p4info.txtpb new file mode 100644 index 00000000000..fdf16790b91 --- /dev/null +++ b/testdata/p4_16_samples_outputs/array2.p4.p4info.txtpb @@ -0,0 +1,6 @@ +# proto-file: p4/config/v1/p4info.proto +# proto-message: p4.config.v1.P4Info + +pkg_info { + arch: "v1model" +} From 54b142cc1cac8d73753828348d2d2be04c348d0a Mon Sep 17 00:00:00 2001 From: Chris Dodd Date: Thu, 30 Jan 2025 07:27:24 +0000 Subject: [PATCH 3/4] Fix/xfail testgen failures, fix lint issues Signed-off-by: Chris Dodd --- .../p4tools/common/core/abstract_execution_state.cpp | 3 --- .../testgen/targets/bmv2/test/BMV2MetadataXfail.cmake | 8 ++++++++ .../modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake | 8 ++++++++ .../testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake | 8 ++++++++ .../modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake | 8 ++++++++ frontends/p4/typeChecking/typeCheckExpr.cpp | 4 ++-- frontends/p4/typeChecking/typeCheckTypes.cpp | 9 ++++++--- 7 files changed, 40 insertions(+), 8 deletions(-) diff --git a/backends/p4tools/common/core/abstract_execution_state.cpp b/backends/p4tools/common/core/abstract_execution_state.cpp index edd5cb7576f..1875e04873f 100644 --- a/backends/p4tools/common/core/abstract_execution_state.cpp +++ b/backends/p4tools/common/core/abstract_execution_state.cpp @@ -158,9 +158,6 @@ std::vector AbstractExecutionState::getFlatFields( for (size_t arrayIndex = 0; arrayIndex < typeStack->getSize(); arrayIndex++) { const auto *newArr = HSIndexToMember::produceStackIndex(stackElementsType, parent, arrayIndex); - BUG_CHECK(stackElementsType->is(), - "Trying to get the flat fields for a non Type_StructLike element : %1%", - stackElementsType); auto subFields = getFlatFields(newArr, validVector); flatFields.insert(flatFields.end(), subFields.begin(), subFields.end()); } diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2MetadataXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2MetadataXfail.cmake index 5803decc815..c110c1d2fc0 100644 --- a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2MetadataXfail.cmake +++ b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2MetadataXfail.cmake @@ -64,6 +64,14 @@ p4tools_add_xfail_reason( loop-3-clause-tricky2.p4 ) +p4tools_add_xfail_reason( + "testgen-p4c-bmv2-metadata" + "Cast failed" + # testgen has issues with arrays + array1.p4 + array2.p4 +) + #################################################################################################### # 3. WONTFIX #################################################################################################### diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake index ff800411663..5bdcfbae3f9 100644 --- a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake +++ b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake @@ -108,6 +108,14 @@ p4tools_add_xfail_reason( loop-3-clause-tricky2.p4 ) +p4tools_add_xfail_reason( + "testgen-p4c-bmv2-ptf" + "Cast failed" + # testgen has issues with arrays + array1.p4 + array2.p4 +) + #################################################################################################### # 3. WONTFIX # These are failures that can not be solved by changing P4Testgen diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake index 37938f56512..f8316c280fa 100644 --- a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake +++ b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake @@ -81,6 +81,14 @@ p4tools_add_xfail_reason( loop-3-clause-tricky2.p4 ) +p4tools_add_xfail_reason( + "testgen-p4c-bmv2-protobuf-ir" + "Cast failed" + # testgen has issues with arrays + array1.p4 + array2.p4 +) + #################################################################################################### # 3. WONTFIX #################################################################################################### diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake index dd146980cab..179825dad3d 100644 --- a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake +++ b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake @@ -108,6 +108,14 @@ p4tools_add_xfail_reason( loop-3-clause-tricky2.p4 ) +p4tools_add_xfail_reason( + "testgen-p4c-bmv2-stf" + "Cast failed" + # testgen has issues with arrays + array1.p4 + array2.p4 +) + #################################################################################################### # 3. WONTFIX # These are failures that can not be solved by changing P4Testgen diff --git a/frontends/p4/typeChecking/typeCheckExpr.cpp b/frontends/p4/typeChecking/typeCheckExpr.cpp index b711c581a0a..3544035e434 100644 --- a/frontends/p4/typeChecking/typeCheckExpr.cpp +++ b/frontends/p4/typeChecking/typeCheckExpr.cpp @@ -1707,8 +1707,8 @@ const IR::Node *TypeInferenceBase::postorder(const IR::Member *expression) { if (auto *stack = type->to()) { auto parser = findContext(); auto eltype = stack->elementType; - if (!eltype->is() && !eltype->is() /* && - !eltype->is() */) { + if (auto sc = eltype->to()) eltype = sc->baseType; + if (!eltype->is() && !eltype->is()) { typeError("%1%: '%2%' can only be used on header stacks", expression, member); return expression; } else if (member == IR::Type_Stack::next || member == IR::Type_Stack::last) { diff --git a/frontends/p4/typeChecking/typeCheckTypes.cpp b/frontends/p4/typeChecking/typeCheckTypes.cpp index 9a9cbb7415e..10f934ca114 100644 --- a/frontends/p4/typeChecking/typeCheckTypes.cpp +++ b/frontends/p4/typeChecking/typeCheckTypes.cpp @@ -425,9 +425,12 @@ const IR::Node *TypeInferenceBase::postorder(const IR::Type_Header *type) { auto canon = setTypeType(type); auto validator = [this](const IR::Type *t) { while (1) { - if (t->is()) t = getTypeType(t->to()->type); - else if (auto *st = t->to()) t = st->elementType; - else break; + if (t->is()) + t = getTypeType(t->to()->type); + else if (auto *st = t->to()) + t = st->elementType; + else + break; } return t->is() || t->is() || (t->is() && onlyBitsOrBitStructs(t)) || t->is() || From 4150286d671adea350937b05fff88436c4fd10d4 Mon Sep 17 00:00:00 2001 From: Chris Dodd Date: Mon, 3 Feb 2025 04:25:18 +0000 Subject: [PATCH 4/4] Clean up typeCheckTypes a bit; another test --- frontends/p4/typeChecking/typeCheckTypes.cpp | 16 +++------ testdata/p4_16_samples/array3.p4 | 30 ++++++++++++++++ .../p4_16_samples_outputs/array3-first.p4 | 27 ++++++++++++++ .../p4_16_samples_outputs/array3-frontend.p4 | 27 ++++++++++++++ .../p4_16_samples_outputs/array3-midend.p4 | 36 +++++++++++++++++++ testdata/p4_16_samples_outputs/array3.p4 | 27 ++++++++++++++ .../p4_16_samples_outputs/array3.p4-stderr | 0 7 files changed, 151 insertions(+), 12 deletions(-) create mode 100644 testdata/p4_16_samples/array3.p4 create mode 100644 testdata/p4_16_samples_outputs/array3-first.p4 create mode 100644 testdata/p4_16_samples_outputs/array3-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/array3-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/array3.p4 create mode 100644 testdata/p4_16_samples_outputs/array3.p4-stderr diff --git a/frontends/p4/typeChecking/typeCheckTypes.cpp b/frontends/p4/typeChecking/typeCheckTypes.cpp index 10f934ca114..c9b5eb12d28 100644 --- a/frontends/p4/typeChecking/typeCheckTypes.cpp +++ b/frontends/p4/typeChecking/typeCheckTypes.cpp @@ -47,6 +47,7 @@ bool hasVarbitsOrUnions(const TypeMap *typeMap, const IR::Type *type) { bool TypeInferenceBase::onlyBitsOrBitStructs(const IR::Type *type) const { // called for a canonical type + while (auto *st = type->to()) type = st->elementType; if (type->is() || type->is() || type->is()) { return true; } else if (auto ht = type->to()) { @@ -424,18 +425,9 @@ const IR::Node *TypeInferenceBase::postorder(const IR::StructField *field) { const IR::Node *TypeInferenceBase::postorder(const IR::Type_Header *type) { auto canon = setTypeType(type); auto validator = [this](const IR::Type *t) { - while (1) { - if (t->is()) - t = getTypeType(t->to()->type); - else if (auto *st = t->to()) - t = st->elementType; - else - break; - } - return t->is() || t->is() || - (t->is() && onlyBitsOrBitStructs(t)) || t->is() || - t->is() || t->is() || - t->is(); + while (t->is()) t = getTypeType(t->to()->type); + return onlyBitsOrBitStructs(t) || t->is() || + t->is() || t->is(); }; validateFields(canon, validator); return type; diff --git a/testdata/p4_16_samples/array3.p4 b/testdata/p4_16_samples/array3.p4 new file mode 100644 index 00000000000..37549afce71 --- /dev/null +++ b/testdata/p4_16_samples/array3.p4 @@ -0,0 +1,30 @@ +control proto

(inout P pkt); +package top

(proto

p); + +struct S { + bit<16> f1; + bit<8>[16] nested_arr; + bit<16> f2; +} + +header data_t { + S[16] arr; + bit<4> w; + bit<4> x; + bit<4> y; + bit<4> z; +} + +struct headers { + data_t data; +} + + +control c(inout headers hdr) { + apply { + hdr.data.arr[0].nested_arr[1] = + hdr.data.arr[2].nested_arr[3]; + } +} + +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/array3-first.p4 b/testdata/p4_16_samples_outputs/array3-first.p4 new file mode 100644 index 00000000000..fe5f1f08e55 --- /dev/null +++ b/testdata/p4_16_samples_outputs/array3-first.p4 @@ -0,0 +1,27 @@ +control proto

(inout P pkt); +package top

(proto

p); +struct S { + bit<16> f1; + bit<8>[16] nested_arr; + bit<16> f2; +} + +header data_t { + S[16] arr; + bit<4> w; + bit<4> x; + bit<4> y; + bit<4> z; +} + +struct headers { + data_t data; +} + +control c(inout headers hdr) { + apply { + hdr.data.arr[0].nested_arr[1] = hdr.data.arr[2].nested_arr[3]; + } +} + +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/array3-frontend.p4 b/testdata/p4_16_samples_outputs/array3-frontend.p4 new file mode 100644 index 00000000000..fe5f1f08e55 --- /dev/null +++ b/testdata/p4_16_samples_outputs/array3-frontend.p4 @@ -0,0 +1,27 @@ +control proto

(inout P pkt); +package top

(proto

p); +struct S { + bit<16> f1; + bit<8>[16] nested_arr; + bit<16> f2; +} + +header data_t { + S[16] arr; + bit<4> w; + bit<4> x; + bit<4> y; + bit<4> z; +} + +struct headers { + data_t data; +} + +control c(inout headers hdr) { + apply { + hdr.data.arr[0].nested_arr[1] = hdr.data.arr[2].nested_arr[3]; + } +} + +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/array3-midend.p4 b/testdata/p4_16_samples_outputs/array3-midend.p4 new file mode 100644 index 00000000000..58b57571e72 --- /dev/null +++ b/testdata/p4_16_samples_outputs/array3-midend.p4 @@ -0,0 +1,36 @@ +control proto

(inout P pkt); +package top

(proto

p); +struct S { + bit<16> f1; + bit<8>[16] nested_arr; + bit<16> f2; +} + +header data_t { + S[16] arr; + bit<4> w; + bit<4> x; + bit<4> y; + bit<4> z; +} + +struct headers { + data_t data; +} + +control c(inout headers hdr) { + @hidden action array3l25() { + hdr.data.arr[0].nested_arr[1] = hdr.data.arr[2].nested_arr[3]; + } + @hidden table tbl_array3l25 { + actions = { + array3l25(); + } + const default_action = array3l25(); + } + apply { + tbl_array3l25.apply(); + } +} + +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/array3.p4 b/testdata/p4_16_samples_outputs/array3.p4 new file mode 100644 index 00000000000..c8d032bb310 --- /dev/null +++ b/testdata/p4_16_samples_outputs/array3.p4 @@ -0,0 +1,27 @@ +control proto

(inout P pkt); +package top

(proto

p); +struct S { + bit<16> f1; + bit<8>[16] nested_arr; + bit<16> f2; +} + +header data_t { + S[16] arr; + bit<4> w; + bit<4> x; + bit<4> y; + bit<4> z; +} + +struct headers { + data_t data; +} + +control c(inout headers hdr) { + apply { + hdr.data.arr[0].nested_arr[1] = hdr.data.arr[2].nested_arr[3]; + } +} + +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/array3.p4-stderr b/testdata/p4_16_samples_outputs/array3.p4-stderr new file mode 100644 index 00000000000..e69de29bb2d