From 1f2c23fc50c5084cdd71b64e03e13bfa1072716b Mon Sep 17 00:00:00 2001 From: Chris Dodd Date: Tue, 28 Mar 2017 14:24:57 -0700 Subject: [PATCH] Allow masked table keys in frontend and bmv2 backend --- backends/bmv2/Makefile.am | 4 -- backends/bmv2/jsonconverter.cpp | 17 +++-- backends/bmv2/midend.cpp | 2 +- backends/p4test/Makefile.am | 3 - frontends/p4/fromv1.0/converters.cpp | 15 ++-- frontends/p4/tableKeyNames.cpp | 11 +++ ir/expression.def | 4 ++ midend/simplifyKey.h | 13 ++++ testdata/p4_14_samples/exact_match_mask1.p4 | 2 +- .../exact_match_mask1-first.p4 | 71 +++++++++++++++++++ .../exact_match_mask1-frontend.p4 | 71 +++++++++++++++++++ .../exact_match_mask1.p4 | 71 +++++++++++++++++++ .../exact_match_mask1.p4-stderr | 0 13 files changed, 261 insertions(+), 23 deletions(-) create mode 100644 testdata/p4_14_samples_outputs/exact_match_mask1-first.p4 create mode 100644 testdata/p4_14_samples_outputs/exact_match_mask1-frontend.p4 create mode 100644 testdata/p4_14_samples_outputs/exact_match_mask1.p4 create mode 100644 testdata/p4_14_samples_outputs/exact_match_mask1.p4-stderr diff --git a/backends/bmv2/Makefile.am b/backends/bmv2/Makefile.am index 688a220e99f..3ebcfe1060b 100644 --- a/backends/bmv2/Makefile.am +++ b/backends/bmv2/Makefile.am @@ -48,7 +48,3 @@ bmv2tests.mk: $(GENTESTS) $(srcdir)/%reldir%/Makefile.am \ $(srcdir)/testdata/p4_14_samples/switch_*/switch.p4 \ $(srcdir)/testdata/p4_16_samples $(srcdir)/testdata/p4_14_samples @$(GENTESTS) $(srcdir) bmv2 $(srcdir)/backends/bmv2/run-bmv2-test.py $^ >$@ - - -#issue #404 -XFAIL_TESTS += bmv2/testdata/p4_14_samples/exact_match_mask1.p4.test diff --git a/backends/bmv2/jsonconverter.cpp b/backends/bmv2/jsonconverter.cpp index d5d5c7d54ac..d29bcc4f38d 100644 --- a/backends/bmv2/jsonconverter.cpp +++ b/backends/bmv2/jsonconverter.cpp @@ -1368,13 +1368,20 @@ JsonConverter::convertTable(const CFG::TableNode* node, BUG_CHECK(mt != nullptr, "%1%: could not find declaration", ke->matchType); auto expr = ke->expression; mpz_class mask; - if (expr->is()) { - auto slice = expr->to(); + if (auto mexp = expr->to()) { + if (mexp->right->is()) { + mask = mexp->right->to()->value; + expr = mexp->left; + } else if (mexp->left->is()) { + mask = mexp->left->to()->value; + expr = mexp->right; + } else { + ::error("%1%: key mask must be a constant", expr); } + } else if (auto slice = expr->to()) { expr = slice->e0; int h = slice->getH(); int l = slice->getL(); - mask = Util::maskFromSlice(h, l); - } + mask = Util::maskFromSlice(h, l); } cstring match_type = mt->name.name; if (mt->name.name == corelib.exactMatch.name) { @@ -1420,7 +1427,7 @@ JsonConverter::convertTable(const CFG::TableNode* node, auto jk = conv->convert(expr); keyelement->emplace("target", jk->to()->get("value")); if (mask != 0) - keyelement->emplace("mask", stringRepr(mask)); + keyelement->emplace("mask", stringRepr(mask, (expr->type->width_bits() + 7) / 8)); else keyelement->emplace("mask", Util::JsonValue::null); tkey->append(keyelement); diff --git a/backends/bmv2/midend.cpp b/backends/bmv2/midend.cpp index 340358f99d8..faab92b87d7 100644 --- a/backends/bmv2/midend.cpp +++ b/backends/bmv2/midend.cpp @@ -150,7 +150,7 @@ MidEnd::MidEnd(CompilerOptions& options) { new P4::SimplifyControlFlow(&refMap, &typeMap), new P4::RemoveActionParameters(&refMap, &typeMap), new P4::SimplifyKey(&refMap, &typeMap, - new P4::NonLeftValue(&refMap, &typeMap)), + new P4::NonMaskLeftValue(&refMap, &typeMap)), new P4::ConstantFolding(&refMap, &typeMap), new P4::StrengthReduction(), new P4::SimplifySelectCases(&refMap, &typeMap, true), // require constant keysets diff --git a/backends/p4test/Makefile.am b/backends/p4test/Makefile.am index df9f623ab07..5d5fd9ec19d 100644 --- a/backends/p4test/Makefile.am +++ b/backends/p4test/Makefile.am @@ -52,6 +52,3 @@ p14_to_16tests.mk: $(GENTESTS) $(srcdir)/%reldir%/Makefile.am \ # This is issue #13 XFAIL_TESTS += \ p4/testdata/p4_16_samples/cast-call.p4.test - -#issue #404 -XFAIL_TESTS += p14_to_16/testdata/p4_14_samples/exact_match_mask1.p4.test diff --git a/frontends/p4/fromv1.0/converters.cpp b/frontends/p4/fromv1.0/converters.cpp index 9977ac4c44c..1890dcb41cf 100644 --- a/frontends/p4/fromv1.0/converters.cpp +++ b/frontends/p4/fromv1.0/converters.cpp @@ -44,15 +44,12 @@ const IR::Node* ExpressionConverter::postorder(IR::Mask* expression) { auto cst = expression->right->to(); mpz_class value = cst->value; auto range = Util::findOnes(value); - if (value != range.value) { - /* FIXME -- can't represent the mask as a single slice -- no way to express this - * valid P4_14 code in P4_16? */ - return expression; } - if (exp->type->is() || range.lowIndex != 0 || - range.highIndex < exp->type->width_bits() - 1U) { - exp = new IR::Slice(Util::SourceInfo(), exp, - new IR::Constant(range.highIndex), new IR::Constant(range.lowIndex)); } - return exp; + if (range.lowIndex == 0 && range.highIndex >= exp->type->width_bits() - 1U) + return exp; + if (value != range.value) + return new IR::BAnd(expression->srcInfo, exp, cst); + return new IR::Slice(Util::SourceInfo(), exp, + new IR::Constant(range.highIndex), new IR::Constant(range.lowIndex)); } const IR::Node* ExpressionConverter::postorder(IR::Constant* expression) { diff --git a/frontends/p4/tableKeyNames.cpp b/frontends/p4/tableKeyNames.cpp index 7c1d40abb87..c7ecd2dfcbf 100644 --- a/frontends/p4/tableKeyNames.cpp +++ b/frontends/p4/tableKeyNames.cpp @@ -57,6 +57,17 @@ class KeyNameGenerator : public Inspector { name.emplace(expression, l + "[" + r + "]"); } + void postorder(const IR::BAnd *expression) override { + if (expression->right->is()) { + if (cstring l = getName(expression->left)) + name.emplace(expression, l); + } else if (expression->left->is()) { + if (cstring r = getName(expression->right)) + name.emplace(expression, r); + } else { + error(expression); } + } + void postorder(const IR::Constant* expression) override { name.emplace(expression, expression->toString()); } diff --git a/ir/expression.def b/ir/expression.def index c6924627a5e..a9ad5758f85 100644 --- a/ir/expression.def +++ b/ir/expression.def @@ -248,11 +248,15 @@ class ArrayIndex : Operation_Binary { class Range : Operation_Binary { stringOp = ".."; precedence = DBPrint::Prec_Low; + Range { if (left && type == left->type && !left->type->is()) + type = new Type_Set(left->type); } } class Mask : Operation_Binary { stringOp = "&&&"; precedence = DBPrint::Prec_Low; + Mask { if (left && type == left->type && !left->type->is()) + type = new Type_Set(left->type); } } class Mux : Operation_Ternary { diff --git a/midend/simplifyKey.h b/midend/simplifyKey.h index 4bbebc8ac6f..cbc3a0c8b41 100644 --- a/midend/simplifyKey.h +++ b/midend/simplifyKey.h @@ -42,6 +42,19 @@ class NonLeftValue : public KeyIsComplex { bool isTooComplex(const IR::Expression* expression) const; }; +// Policy that allows masked lvalues as well as simple lvalues or isValid() +class NonMaskLeftValue : public NonLeftValue { + public: + NonMaskLeftValue(ReferenceMap* refMap, TypeMap* typeMap) : NonLeftValue(refMap, typeMap) {} + bool isTooComplex(const IR::Expression* expression) const { + if (auto mask = expression->to()) { + if (mask->right->is()) + expression = mask->left; + else if (mask->left->is()) + expression = mask->right; } + return NonLeftValue::isTooComplex(expression); } +}; + class TableInsertions { public: std::vector declarations; diff --git a/testdata/p4_14_samples/exact_match_mask1.p4 b/testdata/p4_14_samples/exact_match_mask1.p4 index 2e171e7af39..837cad86f87 100644 --- a/testdata/p4_14_samples/exact_match_mask1.p4 +++ b/testdata/p4_14_samples/exact_match_mask1.p4 @@ -1,5 +1,5 @@ /* -Copyright 2013-present Barefoot Networks, Inc. +Copyright 2013-present Barefoot Networks, Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/testdata/p4_14_samples_outputs/exact_match_mask1-first.p4 b/testdata/p4_14_samples_outputs/exact_match_mask1-first.p4 new file mode 100644 index 00000000000..5d99bfa301f --- /dev/null +++ b/testdata/p4_14_samples_outputs/exact_match_mask1-first.p4 @@ -0,0 +1,71 @@ +#include +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<8> b1; + bit<8> b2; +} + +struct metadata { +} + +struct headers { + @name("data") + data_t data; +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("start") state start { + packet.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("setb1") action setb1(bit<8> val, bit<9> port) { + hdr.data.b1 = val; + standard_metadata.egress_spec = port; + } + @name("noop") action noop() { + } + @name("test1") table test1 { + actions = { + setb1(); + noop(); + @default_only NoAction(); + } + key = { + hdr.data.f1 & 32w0xff00ff: exact @name("hdr.data.f1") ; + } + default_action = NoAction(); + } + apply { + test1.apply(); + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + apply { + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr.data); + } +} + +control verifyChecksum(in headers hdr, inout metadata meta) { + apply { + } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; diff --git a/testdata/p4_14_samples_outputs/exact_match_mask1-frontend.p4 b/testdata/p4_14_samples_outputs/exact_match_mask1-frontend.p4 new file mode 100644 index 00000000000..d549af1dfaf --- /dev/null +++ b/testdata/p4_14_samples_outputs/exact_match_mask1-frontend.p4 @@ -0,0 +1,71 @@ +#include +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<8> b1; + bit<8> b2; +} + +struct metadata { +} + +struct headers { + @name("data") + data_t data; +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("start") state start { + packet.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("setb1") action setb1_0(bit<8> val, bit<9> port) { + hdr.data.b1 = val; + standard_metadata.egress_spec = port; + } + @name("noop") action noop_0() { + } + @name("test1") table test1_0 { + actions = { + setb1_0(); + noop_0(); + @default_only NoAction(); + } + key = { + hdr.data.f1 & 32w0xff00ff: exact @name("hdr.data.f1") ; + } + default_action = NoAction(); + } + apply { + test1_0.apply(); + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + apply { + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr.data); + } +} + +control verifyChecksum(in headers hdr, inout metadata meta) { + apply { + } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; diff --git a/testdata/p4_14_samples_outputs/exact_match_mask1.p4 b/testdata/p4_14_samples_outputs/exact_match_mask1.p4 new file mode 100644 index 00000000000..50e9aa20b14 --- /dev/null +++ b/testdata/p4_14_samples_outputs/exact_match_mask1.p4 @@ -0,0 +1,71 @@ +#include +#include + +header data_t { + bit<32> f1; + bit<32> f2; + bit<16> h1; + bit<8> b1; + bit<8> b2; +} + +struct metadata { +} + +struct headers { + @name("data") + data_t data; +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("start") state start { + packet.extract(hdr.data); + transition accept; + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("setb1") action setb1(bit<8> val, bit<9> port) { + hdr.data.b1 = val; + standard_metadata.egress_spec = port; + } + @name("noop") action noop() { + } + @name("test1") table test1 { + actions = { + setb1; + noop; + @default_only NoAction; + } + key = { + hdr.data.f1 & 32w0xff00ff: exact; + } + default_action = NoAction(); + } + apply { + test1.apply(); + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + apply { + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr.data); + } +} + +control verifyChecksum(in headers hdr, inout metadata meta) { + apply { + } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; diff --git a/testdata/p4_14_samples_outputs/exact_match_mask1.p4-stderr b/testdata/p4_14_samples_outputs/exact_match_mask1.p4-stderr new file mode 100644 index 00000000000..e69de29bb2d