Allow masked table keys in frontend and bmv2 backend

p4lang · Mar 29, 2017 · 1f2c23f · 1f2c23f
1 parent aee4f47
commit 1f2c23f
Show file tree

Hide file tree

Showing 13 changed files with 261 additions and 23 deletions.
diff --git 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
@@ -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<IR::Slice>()) {
-                auto slice = expr->to<IR::Slice>();
+            if (auto mexp = expr->to<IR::BAnd>()) {
+                if (mexp->right->is<IR::Constant>()) {
+                    mask = mexp->right->to<IR::Constant>()->value;
+                    expr = mexp->left;
+                } else if (mexp->left->is<IR::Constant>()) {
+                    mask = mexp->left->to<IR::Constant>()->value;
+                    expr = mexp->right;
+                } else {
+                    ::error("%1%: key mask must be a constant", expr); }
+            } else if (auto slice = expr->to<IR::Slice>()) {
                 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<Util::JsonObject>()->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
@@ -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
@@ -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
@@ -44,15 +44,12 @@ const IR::Node* ExpressionConverter::postorder(IR::Mask* expression) {
     auto cst = expression->right->to<IR::Constant>();
     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<IR::Type::Unknown>() || 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
@@ -57,6 +57,17 @@ class KeyNameGenerator : public Inspector {
         name.emplace(expression, l + "[" + r + "]");
     }
 
+    void postorder(const IR::BAnd *expression) override {
+        if (expression->right->is<IR::Constant>()) {
+            if (cstring l = getName(expression->left))
+                name.emplace(expression, l);
+        } else if (expression->left->is<IR::Constant>()) {
+            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
@@ -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::Unknown>())
+                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::Unknown>())
+                type = new Type_Set(left->type); }
 }
 
 class Mux : Operation_Ternary {

diff --git 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<IR::BAnd>()) {
+            if (mask->right->is<IR::Constant>())
+                expression = mask->left;
+            else if (mask->left->is<IR::Constant>())
+                expression = mask->right; }
+        return NonLeftValue::isTooComplex(expression); }
+};
+
 class TableInsertions {
  public:
     std::vector<const IR::Declaration_Variable*> declarations;

diff --git 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
@@ -0,0 +1,71 @@
+#include <core.p4>
+#include <v1model.p4>
+
+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<data_t>(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<data_t>(hdr.data);
+    }
+}
+
+control verifyChecksum(in headers hdr, inout metadata meta) {
+    apply {
+    }
+}
+
+control computeChecksum(inout headers hdr, inout metadata meta) {
+    apply {
+    }
+}
+
+V1Switch<headers, metadata>(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
@@ -0,0 +1,71 @@
+#include <core.p4>
+#include <v1model.p4>
+
+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<data_t>(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<data_t>(hdr.data);
+    }
+}
+
+control verifyChecksum(in headers hdr, inout metadata meta) {
+    apply {
+    }
+}
+
+control computeChecksum(inout headers hdr, inout metadata meta) {
+    apply {
+    }
+}
+
+V1Switch<headers, metadata>(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
@@ -0,0 +1,71 @@
+#include <core.p4>
+#include <v1model.p4>
+
+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