From 4c07a943706053d44704cc1d590cf178022d22e8 Mon Sep 17 00:00:00 2001 From: mbudiu-vmw Date: Tue, 9 May 2017 15:11:29 -0700 Subject: [PATCH 1/5] Improve support for header unions; part 1 --- backends/ebpf/ebpfType.cpp | 4 +- control-plane/p4RuntimeSerializer.cpp | 2 +- frontends/p4/def_use.cpp | 9 ++- frontends/p4/def_use.h | 93 +++++++++++++---------- frontends/p4/methodInstance.cpp | 5 +- frontends/p4/methodInstance.h | 1 + frontends/p4/resetHeaders.cpp | 2 +- frontends/p4/toP4/toP4.h | 2 +- frontends/p4/typeChecking/typeChecker.cpp | 27 ++++--- frontends/p4/typeChecking/typeChecker.h | 2 +- frontends/p4/validateParsedProgram.cpp | 6 -- frontends/p4/validateParsedProgram.h | 2 - frontends/parsers/p4/p4parser.ypp | 5 +- ir/type.def | 4 +- midend/interpreter.cpp | 4 +- testdata/p4_16_samples/struct.p4 | 13 ++-- 16 files changed, 100 insertions(+), 81 deletions(-) diff --git a/backends/ebpf/ebpfType.cpp b/backends/ebpf/ebpfType.cpp index 8b953b892cf..8e0a52fb309 100644 --- a/backends/ebpf/ebpfType.cpp +++ b/backends/ebpf/ebpfType.cpp @@ -107,7 +107,7 @@ EBPFStructType::EBPFStructType(const IR::Type_StructLike* strct) : kind = "struct"; else if (strct->is()) kind = "struct"; - else if (strct->is()) + else if (strct->is()) kind = "union"; else BUG("Unexpected struct type %1%", strct); @@ -139,7 +139,7 @@ EBPFStructType::declare(CodeBuilder* builder, cstring id, bool asPointer) { void EBPFStructType::emitInitializer(CodeBuilder* builder) { builder->blockStart(); - if (type->is() || type->is()) { + if (type->is() || type->is()) { for (auto f : fields) { builder->emitIndent(); builder->appendFormat(".%s = ", f->field->name.name); diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp index 3fcd91ffc3c..33b144a33c4 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -128,7 +128,7 @@ struct HeaderFieldPath { // Top-level structs and unions don't show up in P4Runtime field names, so we use a // blank path component name. - if (type->is() || type->is()) { + if (type->is() || type->is()) { return HeaderFieldPath::root("", type); } } diff --git a/frontends/p4/def_use.cpp b/frontends/p4/def_use.cpp index d7669168011..f4b993bebca 100644 --- a/frontends/p4/def_use.cpp +++ b/frontends/p4/def_use.cpp @@ -161,7 +161,14 @@ const LocationSet* LocationSet::getField(cstring field) const { for (auto l : locations) { if (l->is()) { auto strct = l->to(); - strct->addField(field, result); + if (field == StorageFactory::validFieldName && strct->isHeaderUnion()) { + // special handling for union.isValid() + for (auto f : strct->fields()) { + f->to()->addField(field, result); + } + } else { + strct->addField(field, result); + } } else { BUG_CHECK(l->is(), "%1%: expected an ArrayLocation", l); auto array = l->to(); diff --git a/frontends/p4/def_use.h b/frontends/p4/def_use.h index e3cfa5152b8..6250905aebd 100644 --- a/frontends/p4/def_use.h +++ b/frontends/p4/def_use.h @@ -25,7 +25,7 @@ namespace P4 { class StorageFactory; class LocationSet; -// Abstraction for something that is has a left value (variable, parameter) +/// Abstraction for something that is has a left value (variable, parameter) class StorageLocation : public IHasDbPrint { public: virtual ~StorageLocation() {} @@ -49,32 +49,35 @@ class StorageLocation : public IHasDbPrint { } cstring toString() const { return name; } - // all locations inside that represent valid bits + /// @returns All locations inside that represent valid bits. const LocationSet* getValidBits() const; virtual void addValidBits(LocationSet* result) const = 0; - // set of locations if we exclude all headers + /// @returns All locations inside if we exclude all headers. const LocationSet* removeHeaders() const; virtual void removeHeaders(LocationSet* result) const = 0; - // all locations inside that represent the last index of an array + /// @returns All locations inside that represent the 'lastIndex' of an array. const LocationSet* getLastIndexField() const; virtual void addLastIndexField(LocationSet* result) const = 0; }; -/* Represents a storage location with a simple type. - It could be either a scalar variable, or a field of a struct, etc. */ +/** Represents a storage location with a simple type or a tuple type. + It could be either a scalar variable, or a field of a struct, etc. */ class BaseLocation : public StorageLocation { public: + // We can use this for tuples because tuples have no field accessors, + // so we treat them as monolithic objects. BaseLocation(const IR::Type* type, cstring name) : StorageLocation(type, name) - { BUG_CHECK(type->is() || type->is() - || type->is() || type->is() - || type-is() || type->is(), + { BUG_CHECK(type->is() || type->is() || + type->is() || type->is() || + type->is() || type-is(), "%1%: unexpected type", type); } void addValidBits(LocationSet*) const override {} void addLastIndexField(LocationSet*) const override {} void removeHeaders(LocationSet* result) const override; }; +/** Represents the locations for element of a struct, header or union */ class StructLocation : public StorageLocation { std::map fieldLocations; friend class StorageFactory; @@ -97,6 +100,9 @@ class StructLocation : public StorageLocation { void addValidBits(LocationSet* result) const override; void removeHeaders(LocationSet* result) const override; void addLastIndexField(LocationSet* result) const override; + bool isHeader() const { return type->is(); } + bool isHeaderUnion() const { return type->is(); } + bool isStruct() const { return type->is(); } }; class ArrayLocation : public StorageLocation { @@ -143,8 +149,8 @@ class StorageFactory { static const cstring indexFieldName; }; -// A set of locations that may be read or written by a computation. -// In general this is a conservative approximation of the actual location set. +/// A set of locations that may be read or written by a computation. +/// In general this is a conservative approximation of the actual location set. class LocationSet : public IHasDbPrint { std::set locations; @@ -163,8 +169,8 @@ class LocationSet : public IHasDbPrint { void add(const StorageLocation* location) { locations.emplace(location); } const LocationSet* join(const LocationSet* other) const; - // express this location set only in terms of BaseLocation; - // e.g., a StructLocation is expanded in all its fields. + /// @returns this location set expressed only in terms of BaseLocation; + /// e.g., a StructLocation is expanded in all its fields. const LocationSet* canonicalize() const; void addCanonical(const StorageLocation* location); std::set::const_iterator begin() const { return locations.cbegin(); } @@ -182,7 +188,7 @@ class LocationSet : public IHasDbPrint { bool isEmpty() const { return locations.empty(); } }; -// For each declaration we keep the associated storage +/// Maps a declaration to its associated storage. class StorageMap { std::map storage; StorageFactory factory; @@ -215,11 +221,11 @@ class StorageMap { } }; -// Indicates a statement in the program. -// The stack is for representing calls: i.e., -// table.apply() -> table -> action +/// Indicates a statement in the program. class ProgramPoint : public IHasDbPrint { - // empty stack represents "beforeStart" (see below) + /// The stack is for representing calls for context-sensitive analyses: i.e., + /// table.apply() -> table -> action. + /// The empty stack represents "beforeStart" (see below). std::vector stack; public: @@ -227,7 +233,7 @@ class ProgramPoint : public IHasDbPrint { ProgramPoint(const ProgramPoint& other) : stack(other.stack) {} explicit ProgramPoint(const IR::Node* node) { stack.push_back(node); } ProgramPoint(const ProgramPoint& context, const IR::Node* node); - static ProgramPoint beforeStart; // a point logically before the program start + static ProgramPoint beforeStart; /// A point logically before the program start. bool operator==(const ProgramPoint& other) const; std::size_t hash() const; void dbprint(std::ostream& out) const { @@ -291,17 +297,17 @@ class ProgramPoints : public IHasDbPrint { { return points.cend(); } }; -// List of definers for each base storage (at a specific program point) +/// List of definers for each base storage (at a specific program point). class Definitions : public IHasDbPrint { - // which program points have written last to each location - // (conservative approximation) + /// Set of program points that have written last to each location + /// (conservative approximation). std::map definitions; public: Definitions() = default; Definitions(const Definitions& other) : definitions(other.definitions) {} Definitions* join(const Definitions* other) const; - // point writes the specified LocationSet + /// Point writes the specified LocationSet. Definitions* writes(ProgramPoint point, const LocationSet* locations) const; void set(const BaseLocation* loc, const ProgramPoints* point) { CHECK_NULL(loc); CHECK_NULL(point); definitions[loc] = point; } @@ -330,9 +336,9 @@ class Definitions : public IHasDbPrint { }; class AllDefinitions : public IHasDbPrint { - // These are the definitions available AFTER each ProgramPoint. - // However, for ProgramPoints representing P4Control, P4Action, and P4Table - // the definitions are BEFORE the ProgramPoint. + /// These are the definitions available AFTER each ProgramPoint. + /// However, for ProgramPoints representing P4Control, P4Action, and P4Table + /// the definitions are BEFORE the ProgramPoint. std::unordered_map atPoint; public: @@ -359,28 +365,31 @@ class AllDefinitions : public IHasDbPrint { } }; -// This does not scan variable initializers, so it must be executed -// after these have been removed. -// Run for each parser and control separately. -// Computes the write set for each expression and statement. -// We are controlling precisely the visit order - to simulate a -// simbolic execution of the program. +/** + * Computes the write set for each expression and statement. + * + * This pass is run for each parser and control separately. It + * controls precisely the visit order --- to simulate a simbolic + * execution of the program. + * + * @pre Must be executed after variable initializers have been removed. + * + */ class ComputeWriteSet : public Inspector { protected: - AllDefinitions* definitions; // result - Definitions* currentDefinitions; // before statement currently processed - Definitions* returnedDefinitions; // set by return statements - Definitions* exitDefinitions; // set by exit statements + AllDefinitions* definitions; /// Result computed by this pass. + Definitions* currentDefinitions; /// Before statement currently processed. + Definitions* returnedDefinitions; /// Definitions after return statements. + Definitions* exitDefinitions; /// Definitions after exit statements. ProgramPoint callingContext; const StorageMap* storageMap; - bool lhs; // if true we are processing an - // expression on the lhs of an - // assignment - // For each expression the location set it writes + /// if true we are processing an expression on the lhs of an assignment + bool lhs; + /// For each expression the location set it writes std::map writes; - // Create new visitor, but with same underlying data structures. - // Needed to visit some program fragments repeatedly. + /// Creates new visitor, but with same underlying data structures. + /// Needed to visit some program fragments repeatedly. ComputeWriteSet(const ComputeWriteSet* source, ProgramPoint context, Definitions* definitions) : definitions(source->definitions), currentDefinitions(definitions), returnedDefinitions(nullptr), exitDefinitions(source->exitDefinitions), diff --git a/frontends/p4/methodInstance.cpp b/frontends/p4/methodInstance.cpp index c7c299c18dc..19e268eace4 100644 --- a/frontends/p4/methodInstance.cpp +++ b/frontends/p4/methodInstance.cpp @@ -49,7 +49,10 @@ MethodInstance::resolve(const IR::MethodCallExpression* mce, ReferenceMap* refMa else BUG("Could not find type for %1%", mem->expr); } - if (basetype->is()) { + if (basetype->is()) { + if (mem->member == IR::Type_Header::isValid) + return new BuiltInMethod(mce, mem->member, mem->expr, mt->to()); + } else if (basetype->is()) { if (mem->member == IR::Type_Header::setValid || mem->member == IR::Type_Header::setInvalid || mem->member == IR::Type_Header::isValid) diff --git a/frontends/p4/methodInstance.h b/frontends/p4/methodInstance.h index a99e07dbda9..29e057b1d00 100644 --- a/frontends/p4/methodInstance.h +++ b/frontends/p4/methodInstance.h @@ -151,6 +151,7 @@ These methods are: - header.setValid(), - header.setInvalid(), - header.isValid(), +- union.isValid(), - stack.push(int), - stack.pop(int) */ diff --git a/frontends/p4/resetHeaders.cpp b/frontends/p4/resetHeaders.cpp index ff9777aca9f..c9a3e27c166 100644 --- a/frontends/p4/resetHeaders.cpp +++ b/frontends/p4/resetHeaders.cpp @@ -23,7 +23,7 @@ void DoResetHeaders::generateResets( const IR::Type* type, const IR::Expression* expr, IR::Vector* resets) { - if (type->is() || type->is()) { + if (type->is() || type->is()) { auto sl = type->to(); for (auto f : sl->fields) { auto ftype = typeMap->getType(f, true); diff --git a/frontends/p4/toP4/toP4.h b/frontends/p4/toP4/toP4.h index d51767c556e..5748a9f5943 100644 --- a/frontends/p4/toP4/toP4.h +++ b/frontends/p4/toP4/toP4.h @@ -126,7 +126,7 @@ class ToP4 : public Inspector { { return process(t, "struct"); } bool preorder(const IR::Type_Header* t) override { return process(t, "header"); } - bool preorder(const IR::Type_Union* t) override + bool preorder(const IR::Type_HeaderUnion* t) override { return process(t, "header_union"); } bool preorder(const IR::Type_Package* t) override; bool preorder(const IR::Type_Parser* t) override; diff --git a/frontends/p4/typeChecking/typeChecker.cpp b/frontends/p4/typeChecking/typeChecker.cpp index e3c96f3598a..f06b04bdf74 100644 --- a/frontends/p4/typeChecking/typeChecker.cpp +++ b/frontends/p4/typeChecking/typeChecker.cpp @@ -457,14 +457,14 @@ const IR::Type* TypeInference::canonicalize(const IR::Type* type) { else canon = str; return canon; - } else if (type->is()) { - auto str = type->to(); + } else if (type->is()) { + auto str = type->to(); auto fields = canonicalizeFields(str); if (fields == nullptr) return nullptr; const IR::Type* canon; if (fields != &str->fields) - canon = new IR::Type_Union(str->srcInfo, str->name, str->annotations, *fields); + canon = new IR::Type_HeaderUnion(str->srcInfo, str->name, str->annotations, *fields); else canon = str; return canon; @@ -1107,7 +1107,7 @@ const IR::Node* TypeInference::postorder(IR::Type_Stack* type) { if (etype == nullptr) return type; - if (!etype->is() && !etype->is()) + if (!etype->is() && !etype->is()) typeError("Header stack %1% used with non-header type %2%", type, etype->toString()); return type; @@ -1168,7 +1168,7 @@ const IR::Node* TypeInference::postorder(IR::Type_Struct* type) { auto canon = setTypeType(type); auto validator = [] (const IR::Type* t) { return t->is() || t->is() || - t->is() || t->is() || + t->is() || t->is() || t->is() || t->is() || t->is() || t->is() || t->is() || @@ -1177,7 +1177,7 @@ const IR::Node* TypeInference::postorder(IR::Type_Struct* type) { return type; } -const IR::Node* TypeInference::postorder(IR::Type_Union *type) { +const IR::Node* TypeInference::postorder(IR::Type_HeaderUnion *type) { auto canon = setTypeType(type); auto validator = [] (const IR::Type* t) { return t->is(); }; validateFields(canon, validator); @@ -2130,7 +2130,7 @@ const IR::Node* TypeInference::postorder(IR::Member* expression) { } if (type->is()) { - if (type->is()) { + if (type->is() || type->is()) { if (member == IR::Type_Header::isValid) { // Built-in method auto type = new IR::Type_Method(IR::Type_Boolean::get(), new IR::ParameterList()); @@ -2140,8 +2140,11 @@ const IR::Node* TypeInference::postorder(IR::Member* expression) { setType(getOriginal(), ctype); setType(expression, ctype); return expression; - } else if (member == IR::Type_Header::setValid || - member == IR::Type_Header::setInvalid) { + } + } + if (type->is()) { + if (member == IR::Type_Header::setValid || + member == IR::Type_Header::setInvalid) { if (!isLeftValue(expression->expr)) ::error("%1%: must be applied to a left-value", expression); // Built-in method @@ -2371,7 +2374,8 @@ bool TypeInference::hasVarbits(const IR::Type_Header* type) const { } void TypeInference::checkEmitType(const IR::Expression* emit, const IR::Type* type) const { - if (type->is() || type->is() || type->is()) + if (type->is() || type->is() || + type->is()) return; if (type->is()) { @@ -2643,7 +2647,8 @@ const IR::Node* TypeInference::postorder(IR::DefaultExpression* expression) { } bool TypeInference::containsHeader(const IR::Type* type) { - if (type->is() || type->is() || type->is()) + if (type->is() || type->is() || + type->is()) return true; if (type->is()) { auto st = type->to(); diff --git a/frontends/p4/typeChecking/typeChecker.h b/frontends/p4/typeChecking/typeChecker.h index 192cb59be2a..4a399a5520d 100644 --- a/frontends/p4/typeChecking/typeChecker.h +++ b/frontends/p4/typeChecking/typeChecker.h @@ -200,7 +200,7 @@ class TypeInference : public Transform { const IR::Node* postorder(IR::Type_Header* type) override; const IR::Node* postorder(IR::Type_Stack* type) override; const IR::Node* postorder(IR::Type_Struct* type) override; - const IR::Node* postorder(IR::Type_Union* type) override; + const IR::Node* postorder(IR::Type_HeaderUnion* type) override; const IR::Node* postorder(IR::Type_Typedef* type) override; const IR::Node* postorder(IR::Type_Specialized* type) override; const IR::Node* postorder(IR::Type_SpecializedCanonical* type) override; diff --git a/frontends/p4/validateParsedProgram.cpp b/frontends/p4/validateParsedProgram.cpp index 89468e38fe2..b95635535a8 100644 --- a/frontends/p4/validateParsedProgram.cpp +++ b/frontends/p4/validateParsedProgram.cpp @@ -54,12 +54,6 @@ void ValidateParsedProgram::postorder(const IR::StructField* f) { ::error("%1%: Illegal field name", f->name); } -/// Unions must have at least one field -void ValidateParsedProgram::postorder(const IR::Type_Union* type) { - if (type->fields.size() == 0) - ::error("%1%: empty union", type); -} - /// Width of a bit<> type is at least 0 /// Width of an int<> type is at least 1 void ValidateParsedProgram::postorder(const IR::Type_Bits* type) { diff --git a/frontends/p4/validateParsedProgram.h b/frontends/p4/validateParsedProgram.h index 0ad42e79a7f..d3388bfa46d 100644 --- a/frontends/p4/validateParsedProgram.h +++ b/frontends/p4/validateParsedProgram.h @@ -31,7 +31,6 @@ namespace P4 { - integer constants have valid types - don't care _ is not used as a name for methods, fields, variables, instances - - unions have at least one field - width of bit<> types is positive - width of int<> types is larger than 1 - no parser state is named 'accept' or 'reject' @@ -62,7 +61,6 @@ class ValidateParsedProgram final : public Inspector { void postorder(const IR::StructField* f) override; void postorder(const IR::ParserState* s) override; void postorder(const IR::P4Table* t) override; - void postorder(const IR::Type_Union* type) override; void postorder(const IR::Type_Bits* type) override; void postorder(const IR::ConstructorCallExpression* expression) override; void postorder(const IR::Declaration_Variable* decl) override; diff --git a/frontends/parsers/p4/p4parser.ypp b/frontends/parsers/p4/p4parser.ypp index 6b317ff30c1..5f4f7c2bbb1 100644 --- a/frontends/parsers/p4/p4parser.ypp +++ b/frontends/parsers/p4/p4parser.ypp @@ -641,7 +641,7 @@ structTypeDeclaration headerUnionDeclaration : optAnnotations HEADER_UNION name { driver.structure->declareType(*$3); } - "{" structFieldList "}" { $$ = new IR::Type_Union(@3, *$3, $1, *$6); } + "{" structFieldList "}" { $$ = new IR::Type_HeaderUnion(@3, *$3, $1, *$6); } ; structFieldList @@ -928,7 +928,8 @@ argument ; expressionList - : expression { $$ = new IR::Vector(); + : /* empty */ { $$ = new IR::Vector(); } + | expression { $$ = new IR::Vector(); $$->push_back($1); } | expressionList "," expression { $$ = $1; $$->push_back($3); } ; diff --git a/ir/type.def b/ir/type.def index c98ecff47a9..f9c2b8be746 100644 --- a/ir/type.def +++ b/ir/type.def @@ -242,10 +242,10 @@ class Type_Struct : Type_StructLike { toString{ return cstring("struct ") + externalName(); } } -class Type_Union : Type_StructLike { +class Type_HeaderUnion : Type_StructLike { #nodbprint toString{ return cstring("header_union ") + externalName(); } - /// this makes some assumptions on padding + // this makes some assumptions on padding int width_bits() const override { int rv = 0; for (auto f : fields) diff --git a/midend/interpreter.cpp b/midend/interpreter.cpp index f9117e092d2..70057270b99 100644 --- a/midend/interpreter.cpp +++ b/midend/interpreter.cpp @@ -84,9 +84,9 @@ unsigned SymbolicValueFactory::getWidth(const IR::Type* type) const { return type->to()->size; if (type->is()) return 1; - if (type->is()) { + if (type->is()) { unsigned width = 0; - for (auto f : type->to()->fields) + for (auto f : type->to()->fields) width = std::max(width, getWidth(f->type)); return width; } diff --git a/testdata/p4_16_samples/struct.p4 b/testdata/p4_16_samples/struct.p4 index 9c765e5209b..e3cf7ab7645 100644 --- a/testdata/p4_16_samples/struct.p4 +++ b/testdata/p4_16_samples/struct.p4 @@ -14,24 +14,23 @@ See the License for the specific language governing permissions and limitations under the License. */ -struct P -{ +struct P { bit<32> f1; bit<32> f2; } -struct T -{ +struct T { int<32> t1; int<32> t2; } -struct S -{ +struct S { T s1; T s2; } +struct Empty {}; + const T t = { 32s10, 32s20 }; const S s = { { 32s15, 32s25}, t }; @@ -41,3 +40,5 @@ const int<32> y = s.s1.t2; const int<32> w = .t.t1; const T t1 = s.s1; +const T t1 = (T)s.s1; +const Empty e = {}; From 4608027888fe75deb8f6c0756d6a4c3d149a92c7 Mon Sep 17 00:00:00 2001 From: mbudiu-vmw Date: Wed, 10 May 2017 15:54:38 -0700 Subject: [PATCH 2/5] def-use analysis for unions --- frontends/p4/def_use.cpp | 17 +++++++++++++++-- frontends/p4/def_use.h | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/frontends/p4/def_use.cpp b/frontends/p4/def_use.cpp index f4b993bebca..f6825ee7d90 100644 --- a/frontends/p4/def_use.cpp +++ b/frontends/p4/def_use.cpp @@ -23,7 +23,7 @@ limitations under the License. namespace P4 { -// name for header valid bit +// internal name for header valid bit; used only locally const cstring StorageFactory::validFieldName = "$valid"; const cstring StorageFactory::indexFieldName = "$lastIndex"; const LocationSet* LocationSet::empty = new LocationSet(); @@ -46,8 +46,21 @@ StorageLocation* StorageFactory::create(const IR::Type* type, cstring name) cons type = typeMap->getTypeType(type, true); // get the canonical version auto st = type->to(); auto result = new StructLocation(type, name); + + // For header unions we will model all of the valid fields + // for all components as a single shared field. The + // reason is that updating one of may change all of the + // other ones. + StorageLocation* globalValid = nullptr; + if (type->is()) + globalValid = create(IR::Type_Boolean::get(), name + "." + validFieldName); + for (auto f : st->fields) { - auto sl = create(f->type, name + "." + f->name); + cstring fieldName = name + "." + f->name; + auto sl = create(f->type, fieldName); + if (globalValid != nullptr) + dynamic_cast(sl)->replaceField( + fieldName + "." + validFieldName, globalValid); result->addField(f->name, sl); } if (st->is()) { diff --git a/frontends/p4/def_use.h b/frontends/p4/def_use.h index 6250905aebd..9130cc78a7b 100644 --- a/frontends/p4/def_use.h +++ b/frontends/p4/def_use.h @@ -84,6 +84,8 @@ class StructLocation : public StorageLocation { void addField(cstring name, StorageLocation* field) { fieldLocations.emplace(name, field); CHECK_NULL(field); } + void replaceField(cstring field, StorageLocation* replacement) + { fieldLocations[field] = replacement; } public: StructLocation(const IR::Type* type, cstring name) : StorageLocation(type, name) { From ed5cc1f64837738f5d9ccbe93a1ec7085bb1c444 Mon Sep 17 00:00:00 2001 From: mbudiu-vmw Date: Wed, 10 May 2017 17:08:31 -0700 Subject: [PATCH 3/5] Front-end support for unions; partial fix for #561 --- testdata/p4_16_errors/issue561-1.p4 | 33 +++++++++++++ testdata/p4_16_errors_outputs/issue561-1.p4 | 23 +++++++++ .../p4_16_errors_outputs/issue561-1.p4-stderr | 18 +++++++ testdata/p4_16_samples/issue561.p4 | 48 +++++++++++++++++++ .../p4_16_samples_outputs/issue561-first.p4 | 34 +++++++++++++ .../issue561-frontend.p4 | 35 ++++++++++++++ .../p4_16_samples_outputs/issue561-midend.p4 | 42 ++++++++++++++++ testdata/p4_16_samples_outputs/issue561.p4 | 34 +++++++++++++ .../p4_16_samples_outputs/issue561.p4-stderr | 9 ++++ .../p4_16_samples_outputs/struct-first.p4 | 4 ++ .../p4_16_samples_outputs/struct-frontend.p4 | 3 ++ testdata/p4_16_samples_outputs/struct.p4 | 4 ++ 12 files changed, 287 insertions(+) create mode 100644 testdata/p4_16_errors/issue561-1.p4 create mode 100644 testdata/p4_16_errors_outputs/issue561-1.p4 create mode 100644 testdata/p4_16_errors_outputs/issue561-1.p4-stderr create mode 100644 testdata/p4_16_samples/issue561.p4 create mode 100644 testdata/p4_16_samples_outputs/issue561-first.p4 create mode 100644 testdata/p4_16_samples_outputs/issue561-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/issue561-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/issue561.p4 create mode 100644 testdata/p4_16_samples_outputs/issue561.p4-stderr diff --git a/testdata/p4_16_errors/issue561-1.p4 b/testdata/p4_16_errors/issue561-1.p4 new file mode 100644 index 00000000000..44af3b837a4 --- /dev/null +++ b/testdata/p4_16_errors/issue561-1.p4 @@ -0,0 +1,33 @@ +/* +Copyright 2017 VMware, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +header H1 { bit<32> f; } +header H2 { bit<32> g; } + +header_union U { + H1 h1; + H2 h2; +} + +control ct(); +package top(ct _ct); + +control c() { + apply { + U u = { { 10 }, { 20 } }; // illegal to initialize unions + u.setValid(); // no such method + } +} +top(c()) main; \ No newline at end of file diff --git a/testdata/p4_16_errors_outputs/issue561-1.p4 b/testdata/p4_16_errors_outputs/issue561-1.p4 new file mode 100644 index 00000000000..c89e86bc7bc --- /dev/null +++ b/testdata/p4_16_errors_outputs/issue561-1.p4 @@ -0,0 +1,23 @@ +header H1 { + bit<32> f; +} + +header H2 { + bit<32> g; +} + +header_union U { + H1 h1; + H2 h2; +} + +control ct(); +package top(ct _ct); +control c() { + apply { + U u = { { 10 }, { 20 } }; + u.setValid(); + } +} + +top(c()) main; diff --git a/testdata/p4_16_errors_outputs/issue561-1.p4-stderr b/testdata/p4_16_errors_outputs/issue561-1.p4-stderr new file mode 100644 index 00000000000..98220cd5373 --- /dev/null +++ b/testdata/p4_16_errors_outputs/issue561-1.p4-stderr @@ -0,0 +1,18 @@ +../testdata/p4_16_errors/issue561-1.p4(29): error: u: Cannot unify Tuple(2) to header_union U + U u = { { 10 }, { 20 } }; // illegal to initialize unions + ^^^^^^^^^^^^^^^^^^^^^^^^^ +../testdata/p4_16_errors/issue561-1.p4(29) + U u = { { 10 }, { 20 } }; // illegal to initialize unions + ^^^^^^^^^^^^^^^^^^ +../testdata/p4_16_errors/issue561-1.p4(19) +header_union U { + ^ +../testdata/p4_16_errors/issue561-1.p4(19): error: Structure header_union U does not have a field setValid +header_union U { + ^ +../testdata/p4_16_errors/issue561-1.p4(30) + u.setValid(); // no such method + ^^^^^^^^ +../testdata/p4_16_errors/issue561-1.p4(30): error: Could not find type of u.setValid + u.setValid(); // no such method + ^^^^^^^^^^ diff --git a/testdata/p4_16_samples/issue561.p4 b/testdata/p4_16_samples/issue561.p4 new file mode 100644 index 00000000000..3600cda868b --- /dev/null +++ b/testdata/p4_16_samples/issue561.p4 @@ -0,0 +1,48 @@ +/* +Copyright 2017 VMware, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +header H1 { bit<32> f; } +header H2 { bit<32> g; } + +header_union U { + H1 h1; + H2 h2; +} + +control ct(out bit<32> b); +package top(ct _ct); + +control c(out bit<32> x) { + apply { + U u; + U[2] u2; + + bool b = u.isValid(); + b = b || u.h1.isValid(); + + x = u.h1.f + u.h2.g; + u.h1.setValid(); + u.h1.f = 0; + x = x + u.h1.f; + + u.h2.g = 0; + x = x + u.h2.g; + + u2[0].h1.setValid(); + u2[0].h1.f = 2; + x = x + u2[1].h2.g + u2[0].h1.f; + } +} +top(c()) main; \ No newline at end of file diff --git a/testdata/p4_16_samples_outputs/issue561-first.p4 b/testdata/p4_16_samples_outputs/issue561-first.p4 new file mode 100644 index 00000000000..0b367698c3e --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue561-first.p4 @@ -0,0 +1,34 @@ +header H1 { + bit<32> f; +} + +header H2 { + bit<32> g; +} + +header_union U { + H1 h1; + H2 h2; +} + +control ct(out bit<32> b); +package top(ct _ct); +control c(out bit<32> x) { + apply { + U u; + U[2] u2; + bool b = u.isValid(); + b = b || u.h1.isValid(); + x = u.h1.f + u.h2.g; + u.h1.setValid(); + u.h1.f = 32w0; + x = x + u.h1.f; + u.h2.g = 32w0; + x = x + u.h2.g; + u2[0].h1.setValid(); + u2[0].h1.f = 32w2; + x = x + u2[1].h2.g + u2[0].h1.f; + } +} + +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/issue561-frontend.p4 b/testdata/p4_16_samples_outputs/issue561-frontend.p4 new file mode 100644 index 00000000000..f3f1cb92e14 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue561-frontend.p4 @@ -0,0 +1,35 @@ +header H1 { + bit<32> f; +} + +header H2 { + bit<32> g; +} + +header_union U { + H1 h1; + H2 h2; +} + +control ct(out bit<32> b); +package top(ct _ct); +control c(out bit<32> x) { + U u_0; + U[2] u2_0; + bool b_0; + apply { + b_0 = u_0.isValid(); + u_0.h1.isValid(); + x = u_0.h1.f + u_0.h2.g; + u_0.h1.setValid(); + u_0.h1.f = 32w0; + x = x + u_0.h1.f; + u_0.h2.g = 32w0; + x = x + u_0.h2.g; + u2_0[0].h1.setValid(); + u2_0[0].h1.f = 32w2; + x = x + u2_0[1].h2.g + u2_0[0].h1.f; + } +} + +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/issue561-midend.p4 b/testdata/p4_16_samples_outputs/issue561-midend.p4 new file mode 100644 index 00000000000..58e6444a17e --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue561-midend.p4 @@ -0,0 +1,42 @@ +header H1 { + bit<32> f; +} + +header H2 { + bit<32> g; +} + +header_union U { + H1 h1; + H2 h2; +} + +control ct(out bit<32> b); +package top(ct _ct); +control c(out bit<32> x) { + U u; + U[2] u2; + @hidden action act() { + u.h1.isValid(); + x = u.h1.f + u.h2.g; + u.h1.setValid(); + u.h1.f = 32w0; + x = x + u.h1.f; + u.h2.g = 32w0; + x = x + u.h2.g; + u2[0].h1.setValid(); + u2[0].h1.f = 32w2; + x = x + u2[1].h2.g + u2[0].h1.f; + } + @hidden table tbl_act { + actions = { + act(); + } + const default_action = act(); + } + apply { + tbl_act.apply(); + } +} + +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/issue561.p4 b/testdata/p4_16_samples_outputs/issue561.p4 new file mode 100644 index 00000000000..694b0cc066c --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue561.p4 @@ -0,0 +1,34 @@ +header H1 { + bit<32> f; +} + +header H2 { + bit<32> g; +} + +header_union U { + H1 h1; + H2 h2; +} + +control ct(out bit<32> b); +package top(ct _ct); +control c(out bit<32> x) { + apply { + U u; + U[2] u2; + bool b = u.isValid(); + b = b || u.h1.isValid(); + x = u.h1.f + u.h2.g; + u.h1.setValid(); + u.h1.f = 0; + x = x + u.h1.f; + u.h2.g = 0; + x = x + u.h2.g; + u2[0].h1.setValid(); + u2[0].h1.f = 2; + x = x + u2[1].h2.g + u2[0].h1.f; + } +} + +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/issue561.p4-stderr b/testdata/p4_16_samples_outputs/issue561.p4-stderr new file mode 100644 index 00000000000..d0431627644 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue561.p4-stderr @@ -0,0 +1,9 @@ +../testdata/p4_16_samples/issue561.p4(35): warning: u.h1.f may be uninitialized + x = u.h1.f + u.h2.g; + ^^^^^^ +../testdata/p4_16_samples/issue561.p4(35): warning: u.h2.g may be uninitialized + x = u.h1.f + u.h2.g; + ^^^^^^ +../testdata/p4_16_samples/issue561.p4(45): warning: [].h2.g may be uninitialized + x = x + u2[1].h2.g + u2[0].h1.f; + ^^^^^^^^^^ diff --git a/testdata/p4_16_samples_outputs/struct-first.p4 b/testdata/p4_16_samples_outputs/struct-first.p4 index a014d097f0e..ba7c33fe8ba 100644 --- a/testdata/p4_16_samples_outputs/struct-first.p4 +++ b/testdata/p4_16_samples_outputs/struct-first.p4 @@ -13,9 +13,13 @@ struct S { T s2; } +struct Empty { +} + const T t = { 32s10, 32s20 }; const S s = { { 32s15, 32s25 }, { 32s10, 32s20 } }; const int<32> x = 32s10; const int<32> y = 32s25; const int<32> w = 32s10; const T t1 = { 32s15, 32s25 }; +const Empty e = { }; diff --git a/testdata/p4_16_samples_outputs/struct-frontend.p4 b/testdata/p4_16_samples_outputs/struct-frontend.p4 index 2626f6c84f7..6b92b64f37a 100644 --- a/testdata/p4_16_samples_outputs/struct-frontend.p4 +++ b/testdata/p4_16_samples_outputs/struct-frontend.p4 @@ -13,3 +13,6 @@ struct S { T s2; } +struct Empty { +} + diff --git a/testdata/p4_16_samples_outputs/struct.p4 b/testdata/p4_16_samples_outputs/struct.p4 index 68f47910ef7..104dadfb65e 100644 --- a/testdata/p4_16_samples_outputs/struct.p4 +++ b/testdata/p4_16_samples_outputs/struct.p4 @@ -13,9 +13,13 @@ struct S { T s2; } +struct Empty { +} + const T t = { 32s10, 32s20 }; const S s = { { 32s15, 32s25 }, t }; const int<32> x = t.t1; const int<32> y = s.s1.t2; const int<32> w = .t.t1; const T t1 = s.s1; +const Empty e = { }; From eb3abd1f6e2553a38833cf683baf0118d5806a7f Mon Sep 17 00:00:00 2001 From: mbudiu-vmw Date: Thu, 11 May 2017 12:03:40 -0700 Subject: [PATCH 4/5] cpplint fix --- frontends/p4/def_use.h | 1 + 1 file changed, 1 insertion(+) diff --git a/frontends/p4/def_use.h b/frontends/p4/def_use.h index 9130cc78a7b..e1390d8c7e0 100644 --- a/frontends/p4/def_use.h +++ b/frontends/p4/def_use.h @@ -86,6 +86,7 @@ class StructLocation : public StorageLocation { { fieldLocations.emplace(name, field); CHECK_NULL(field); } void replaceField(cstring field, StorageLocation* replacement) { fieldLocations[field] = replacement; } + public: StructLocation(const IR::Type* type, cstring name) : StorageLocation(type, name) { From 28fdc3f7e614e023075925e78b6973cda9e468c7 Mon Sep 17 00:00:00 2001 From: mbudiu-vmw Date: Fri, 12 May 2017 13:35:43 -0700 Subject: [PATCH 5/5] Fix typo/bug --- frontends/p4/def_use.h | 3 ++- testdata/p4_16_samples/struct.p4 | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontends/p4/def_use.h b/frontends/p4/def_use.h index e1390d8c7e0..daed3adfc91 100644 --- a/frontends/p4/def_use.h +++ b/frontends/p4/def_use.h @@ -70,7 +70,8 @@ class BaseLocation : public StorageLocation { StorageLocation(type, name) { BUG_CHECK(type->is() || type->is() || type->is() || type->is() || - type->is() || type-is(), + type->is() || type->is() || + type->is(), "%1%: unexpected type", type); } void addValidBits(LocationSet*) const override {} void addLastIndexField(LocationSet*) const override {} diff --git a/testdata/p4_16_samples/struct.p4 b/testdata/p4_16_samples/struct.p4 index e3cf7ab7645..f1a1e0e2fda 100644 --- a/testdata/p4_16_samples/struct.p4 +++ b/testdata/p4_16_samples/struct.p4 @@ -40,5 +40,4 @@ const int<32> y = s.s1.t2; const int<32> w = .t.t1; const T t1 = s.s1; -const T t1 = (T)s.s1; const Empty e = {};