From d50facb5e5fa8dcf3da03bc94842d7facf05639e Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Tue, 1 May 2018 14:38:27 -0700 Subject: [PATCH] Fix generation of P4Info for v1model registers Use declaration instance type instead of evaluated block instance type. The evaluated block instance type gives us a Type_SpecializedCanonical node which doesn't use Type_Name and therefore violates pre-condition for typeSpecConverter. --- control-plane/p4RuntimeSerializer.cpp | 13 ++++----- control-plane/typeSpecConverter.cpp | 4 +++ test/gtest/p4runtime.cpp | 38 ++++++++++++++++++--------- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp index e891a1aa888..02cf5ae025f 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -196,7 +196,7 @@ struct Register { const TypeMap* typeMap, p4::P4TypeInfo* p4RtTypeInfo) { CHECK_NULL(instance); - auto declaration = instance->node->to(); + auto declaration = instance->node->to(); auto size = instance->getParameterValue("size")->to(); if (!size->is()) { @@ -210,13 +210,14 @@ struct Register { } // retrieve type parameter for the register instance and convert it to p4::P4DataTypeSpec - BUG_CHECK(instance->instanceType->is(), - "%1%: expected Type_SpecializedCanonical", instance->instanceType); - auto instanceType = instance->instanceType->to(); - BUG_CHECK(instanceType->arguments->size() == 1, + BUG_CHECK(declaration->type->is(), + "%1%: expected Type_Specialized", declaration->type); + auto type = declaration->type->to(); + BUG_CHECK(type->arguments->size() == 1, "%1%: expected one type argument", instance); - auto typeArg = instanceType->arguments->at(0); + auto typeArg = type->arguments->at(0); auto typeSpec = TypeSpecConverter::convert(typeMap, refMap, typeArg, p4RtTypeInfo); + CHECK_NULL(typeSpec); return Register{declaration->controlPlaneName(), declaration->to(), diff --git a/control-plane/typeSpecConverter.cpp b/control-plane/typeSpecConverter.cpp index 2a133af00da..2232b95f77a 100644 --- a/control-plane/typeSpecConverter.cpp +++ b/control-plane/typeSpecConverter.cpp @@ -101,6 +101,7 @@ bool TypeSpecConverter::preorder(const IR::Type_Tuple* type) { for (auto cType : type->components) { visit(cType); auto cTypeSpec = map.at(cType); + CHECK_NULL(cTypeSpec); auto member = tupleTypeSpec->add_members(); member->CopyFrom(*cTypeSpec); } @@ -146,6 +147,7 @@ bool TypeSpecConverter::preorder(const IR::Type_Struct* type) { auto fType = f->type; visit(fType); auto fTypeSpec = map.at(fType); + CHECK_NULL(fTypeSpec); auto member = structTypeSpec->add_members(); member->set_name(f->controlPlaneName()); member->mutable_type_spec()->CopyFrom(*fTypeSpec); @@ -167,6 +169,7 @@ bool TypeSpecConverter::preorder(const IR::Type_Header* type) { auto fType = f->type; visit(fType); auto fTypeSpec = map.at(fType); + CHECK_NULL(fTypeSpec); BUG_CHECK(fTypeSpec->has_bitstring(), "Only bistring fields expected in header type declaration %1%", type); auto member = headerTypeSpec->add_members(); @@ -190,6 +193,7 @@ bool TypeSpecConverter::preorder(const IR::Type_HeaderUnion* type) { auto fType = f->type; visit(fType); auto fTypeSpec = map.at(fType); + CHECK_NULL(fTypeSpec); BUG_CHECK(fTypeSpec->has_header(), "Only header fields expected in header union declaration %1%", type); auto member = headerUnionTypeSpec->add_members(); diff --git a/test/gtest/p4runtime.cpp b/test/gtest/p4runtime.cpp index 2c4f25e9168..bb45e4f6101 100644 --- a/test/gtest/p4runtime.cpp +++ b/test/gtest/p4runtime.cpp @@ -859,8 +859,11 @@ TEST_F(P4Runtime, Register) { control ingress(inout Headers h, inout Metadata m, inout standard_metadata_t sm) { @my_anno("This is an annotation!") - register, bit<8> > >(128) my_register; - apply { my_register.write(32w10, {16w1, 8w2}); } } + register, bit<8> > >(128) my_register_1; + register
(128) my_register_2; + apply { + my_register_1.write(32w10, {16w1, 8w2}); + my_register_2.write(32w10, h.h); } } V1Switch(parse(), verifyChecksum(), ingress(), egress(), computeChecksum(), deparse()) main; )")); @@ -868,16 +871,27 @@ TEST_F(P4Runtime, Register) { ASSERT_TRUE(test); EXPECT_EQ(0u, ::diagnosticCount()); - auto register_ = findRegister(*test, "ingress.my_register"); - ASSERT_TRUE(register_ != nullptr); - EXPECT_EQ(unsigned(P4Ids::REGISTER), register_->preamble().id() >> 24); - const auto& annotations = register_->preamble().annotations(); - ASSERT_EQ(1, annotations.size()); - EXPECT_EQ("@my_anno(\"This is an annotation!\")", annotations.Get(0)); - EXPECT_EQ(128, register_->size()); - const auto& typeSpec = register_->type_spec(); - ASSERT_TRUE(typeSpec.has_tuple()); - EXPECT_EQ(2, typeSpec.tuple().members_size()); + { // type parameter is tuple + auto register_ = findRegister(*test, "ingress.my_register_1"); + ASSERT_TRUE(register_ != nullptr); + EXPECT_EQ(unsigned(P4Ids::REGISTER), register_->preamble().id() >> 24); + const auto& annotations = register_->preamble().annotations(); + ASSERT_EQ(1, annotations.size()); + EXPECT_EQ("@my_anno(\"This is an annotation!\")", annotations.Get(0)); + EXPECT_EQ(128, register_->size()); + const auto& typeSpec = register_->type_spec(); + ASSERT_TRUE(typeSpec.has_tuple()); + EXPECT_EQ(2, typeSpec.tuple().members_size()); + } + { // type parameter is header + auto register_ = findRegister(*test, "ingress.my_register_2"); + ASSERT_TRUE(register_ != nullptr); + EXPECT_EQ(unsigned(P4Ids::REGISTER), register_->preamble().id() >> 24); + EXPECT_EQ(128, register_->size()); + const auto& typeSpec = register_->type_spec(); + ASSERT_TRUE(typeSpec.has_header()); + EXPECT_EQ("Header", typeSpec.header().name()); + } } class P4RuntimeDataTypeSpec : public P4Runtime {