Fix for issue #1237 (#1240)

p4lang · May 1, 2018 · 9724fe8 · 9724fe8
1 parent 27477d7
commit 9724fe8
Show file tree

Hide file tree

Showing 12 changed files with 415 additions and 8 deletions.
diff --git a/frontends/p4/checkConstants.cpp b/frontends/p4/checkConstants.cpp
@@ -33,10 +33,14 @@ void DoCheckConstants::postorder(const IR::MethodCallExpression* expression) {
     }
 }
 
+void DoCheckConstants::postorder(const IR::KeyElement* key) {
+    if (key->expression->is<IR::Literal>())
+        ::warning("%1%: Constant key field", key->expression);
+}
+
 void DoCheckConstants::postorder(const IR::P4Table* table) {
     // This will print an error if the property exists and is not an integer
     (void)table->getSizeProperty();
 }
 
-
 }  // namespace P4
diff --git a/frontends/p4/checkConstants.h b/frontends/p4/checkConstants.h
@@ -36,6 +36,7 @@ class DoCheckConstants : public Inspector {
     }
 
     void postorder(const IR::MethodCallExpression* expr) override;
+    void postorder(const IR::KeyElement* expr) override;
     void postorder(const IR::P4Table* table) override;
 };
 

diff --git a/frontends/p4/fromv1.0/converters.cpp b/frontends/p4/fromv1.0/converters.cpp
@@ -43,12 +43,18 @@ const IR::Node* ExpressionConverter::postorder(IR::Mask* expression) {
     auto exp = expression->left;
     auto cst = expression->right->to<IR::Constant>();
     mpz_class value = cst->value;
+    if (value == 0) {
+        ::warning("%1%: zero mask", expression->right);
+        return cst;
+    }
     auto range = Util::findOnes(value);
     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(exp, new IR::Constant(range.highIndex), new IR::Constant(range.lowIndex));
+    return new IR::Slice(exp,
+                         new IR::Constant(expression->srcInfo, range.highIndex),
+                         new IR::Constant(expression->srcInfo, range.lowIndex));
 }
 
 const IR::Node* ExpressionConverter::postorder(IR::Constant* expression) {

diff --git a/frontends/parsers/parserDriver.cpp b/frontends/parsers/parserDriver.cpp
@@ -218,7 +218,7 @@ V1ParserDriver::parse(FILE* in, const char* sourceFile,
 IR::Constant* V1ParserDriver::constantFold(IR::Expression* expr) {
     IR::Node* node(expr);
     auto rv = node->apply(P4::DoConstantFolding(nullptr, nullptr))->to<IR::Constant>();
-    return rv ? new IR::Constant(rv->type, rv->value, rv->base) : nullptr;
+    return rv ? new IR::Constant(rv->srcInfo, rv->type, rv->value, rv->base) : nullptr;
 }
 
 IR::Vector<IR::Expression>

diff --git a/ir/expression.cpp b/ir/expression.cpp
@@ -18,12 +18,12 @@ limitations under the License.
 #include "dbprint.h"
 #include "lib/gmputil.h"
 
-const IR::Expression *IR::Slice::make(const IR::Expression *e, int lo, int hi) {
+const IR::Expression *IR::Slice::make(const IR::Expression *e, unsigned lo, unsigned hi) {
     if (auto k = e->to<IR::Constant>()) {
         auto rv = ((*k >> lo) & IR::Constant((1U << (hi-lo+1)) - 1)).clone();
         rv->type = IR::Type::Bits::get(hi-lo+1);
         return rv; }
-    if (auto src_width = e->type->width_bits()) {
+    if (auto src_width = (unsigned)e->type->width_bits()) {
         if (lo >= src_width)
             return new IR::Constant(IR::Type::Bits::get(hi-lo+1), 0);
         if (hi >= src_width)

diff --git a/ir/expression.def b/ir/expression.def
@@ -157,6 +157,7 @@ class Constant : Literal {
 #end
     bool fitsInt() const { return value.fits_sint_p(); }
     bool fitsLong() const { return value.fits_slong_p(); }
+    bool fitsUint() const { return value.fits_uint_p(); }
     long asLong() const {
         if (!fitsLong())
             ::error("%1%: Value too large", this);
@@ -165,6 +166,11 @@ class Constant : Literal {
         if (!fitsInt())
             ::error("%1%: Value too large", this);
         return value.get_si(); }
+    unsigned asUnsigned() const {
+        if (!fitsUint())
+            ::error("%1%: Value too large", this);
+        return value.get_ui();
+    }
     // The following operators are only used in special circumstances.
     // They do not look at the type when operating.  There are separate
     // implementations of these computations when doing proper constant folding.
@@ -218,8 +224,8 @@ class Slice : Operation_Ternary {
     stringOp = "[:]";
     toString{ return e0->toString() + "[" + e1->toString() + ":" + e2->toString() + "]"; }
     // After type checking e1 and e2 will be constants
-    int getH() const { return e1->to<IR::Constant>()->asInt(); }
-    int getL() const { return e2->to<IR::Constant>()->asInt(); }
+    unsigned getH() const { return e1->to<IR::Constant>()->asUnsigned(); }
+    unsigned getL() const { return e2->to<IR::Constant>()->asUnsigned(); }
     Slice(Expression a, int hi, int lo)
     : Operation_Ternary(IR::Type::Bits::get(hi-lo+1), a, new Constant(hi), new Constant(lo)) {}
     Slice(Util::SourceInfo si, Expression a, int hi, int lo)
@@ -228,7 +234,7 @@ class Slice : Operation_Ternary {
         if (type->is<Type::Unknown>() && e1 && e1->is<Constant>() && e2 && e2->is<Constant>())
             type = IR::Type::Bits::get(getH() - getL() + 1); }
     // make a slice, folding slices on slices and slices on constants automatically
-    static Expression make(Expression a, int hi, int lo);
+    static Expression make(Expression a, unsigned hi, unsigned lo);
 }
 
 class Member : Operation_Unary {

diff --git a/testdata/p4_14_samples/issue1237.p4 b/testdata/p4_14_samples/issue1237.p4
@@ -0,0 +1,47 @@
+header_type affairs {
+  fields {
+    flaccidly : 48;
+    sleeting : 16;
+  }
+}
+
+header_type backs {
+  fields {
+    sharps : 16;
+    peptides : 16;
+    whirs : 16;
+    breasted : 16;
+    browning : 16;
+    jews : 32;
+  }
+}
+
+header affairs kilometer;
+
+header backs expressivenesss;
+
+parser start {
+  extract(kilometer);
+  extract(expressivenesss);
+  return ingress;
+}
+
+action mooneys() {
+  subtract_from_field(expressivenesss.breasted, expressivenesss.peptides);
+  subtract_from_field(kilometer.sleeting, 7);
+}
+
+table conceptualization {
+  reads {
+    kilometer : valid;
+    kilometer.flaccidly mask 0 : lpm;
+    expressivenesss : valid;
+  }
+  actions {
+    mooneys;
+  }
+}
+
+control ingress {
+  apply(conceptualization);
+}
diff --git a/testdata/p4_14_samples_outputs/issue1237-first.p4 b/testdata/p4_14_samples_outputs/issue1237-first.p4
@@ -0,0 +1,81 @@
+#include <core.p4>
+#include <v1model.p4>
+
+@name("backs") header backs_0 {
+    bit<16> sharps;
+    bit<16> peptides;
+    bit<16> whirs;
+    bit<16> breasted;
+    bit<16> browning;
+    bit<32> jews;
+}
+
+@name("affairs") header affairs_0 {
+    bit<48> flaccidly;
+    bit<16> sleeting;
+}
+
+struct metadata {
+}
+
+struct headers {
+    @name(".expressivenesss") 
+    backs_0   expressivenesss;
+    @name(".kilometer") 
+    affairs_0 kilometer;
+}
+
+parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
+    @name(".start") state start {
+        packet.extract<affairs_0>(hdr.kilometer);
+        packet.extract<backs_0>(hdr.expressivenesss);
+        transition accept;
+    }
+}
+
+control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
+    @name(".mooneys") action mooneys() {
+        hdr.expressivenesss.breasted = hdr.expressivenesss.breasted - hdr.expressivenesss.peptides;
+        hdr.kilometer.sleeting = hdr.kilometer.sleeting + 16w65529;
+    }
+    @name(".conceptualization") table conceptualization {
+        actions = {
+            mooneys();
+            @defaultonly NoAction();
+        }
+        key = {
+            hdr.kilometer.isValid()      : exact @name("kilometer.$valid$") ;
+            48w0                         : lpm @name("0") ;
+            hdr.expressivenesss.isValid(): exact @name("expressivenesss.$valid$") ;
+        }
+        default_action = NoAction();
+    }
+    apply {
+        conceptualization.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<affairs_0>(hdr.kilometer);
+        packet.emit<backs_0>(hdr.expressivenesss);
+    }
+}
+
+control verifyChecksum(inout 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/issue1237-frontend.p4 b/testdata/p4_14_samples_outputs/issue1237-frontend.p4
@@ -0,0 +1,83 @@
+#include <core.p4>
+#include <v1model.p4>
+
+@name("backs") header backs_0 {
+    bit<16> sharps;
+    bit<16> peptides;
+    bit<16> whirs;
+    bit<16> breasted;
+    bit<16> browning;
+    bit<32> jews;
+}
+
+@name("affairs") header affairs_0 {
+    bit<48> flaccidly;
+    bit<16> sleeting;
+}
+
+struct metadata {
+}
+
+struct headers {
+    @name(".expressivenesss") 
+    backs_0   expressivenesss;
+    @name(".kilometer") 
+    affairs_0 kilometer;
+}
+
+parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
+    @name(".start") state start {
+        packet.extract<affairs_0>(hdr.kilometer);
+        packet.extract<backs_0>(hdr.expressivenesss);
+        transition accept;
+    }
+}
+
+control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
+    @name(".NoAction") action NoAction_0() {
+    }
+    @name(".mooneys") action mooneys_0() {
+        hdr.expressivenesss.breasted = hdr.expressivenesss.breasted - hdr.expressivenesss.peptides;
+        hdr.kilometer.sleeting = hdr.kilometer.sleeting + 16w65529;
+    }
+    @name(".conceptualization") table conceptualization {
+        actions = {
+            mooneys_0();
+            @defaultonly NoAction_0();
+        }
+        key = {
+            hdr.kilometer.isValid()      : exact @name("kilometer.$valid$") ;
+            48w0                         : lpm @name("0") ;
+            hdr.expressivenesss.isValid(): exact @name("expressivenesss.$valid$") ;
+        }
+        default_action = NoAction_0();
+    }
+    apply {
+        conceptualization.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<affairs_0>(hdr.kilometer);
+        packet.emit<backs_0>(hdr.expressivenesss);
+    }
+}
+
+control verifyChecksum(inout 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;
+
-Original file line number
+Diff line change
@@ Expand Up / @@ -36,6 +36,7 @@ class DoCheckConstants : public Inspector { @@
         }
         void postorder(const IR::MethodCallExpression* expr) override;
+        void postorder(const IR::KeyElement* expr) override;
         void postorder(const IR::P4Table* table) override;
     };
@@ Expand Down @@