Fix for issue #1478 (#1503)

* Fix for issue #1478
* Remove table size when it is not needed

backends/bmv2/simple_switch/midend.cpp

@@ -28,6 +28,7 @@ limitations under the License.
 #include "frontends/p4/uniqueNames.h"
 #include "frontends/p4/unusedDeclarations.h"
 #include "midend/actionSynthesis.h"
+#include "midend/checkSize.h"
 #include "midend/complexComparison.h"
 #include "midend/convertEnums.h"
 #include "midend/copyStructures.h"
@@ -58,6 +59,7 @@ SimpleSwitchMidEnd::SimpleSwitchMidEnd(CompilerOptions& options) : MidEnd(option
  auto evaluator = new P4::EvaluatorPass(&refMap, &typeMap);
  auto convertEnums = new P4::ConvertEnums(&refMap, &typeMap, new EnumOn32Bits("v1model.p4"));
  addPasses({
+ new P4::CheckTableSize(),
  new P4::EliminateNewtype(&refMap, &typeMap),
  new P4::EliminateSerEnums(&refMap, &typeMap),
  new P4::RemoveActionParameters(&refMap, &typeMap),

ir/indexed_vector.h

@@ -79,7 +79,7 @@ class IndexedVector : public Vector<T> {
  explicit IndexedVector(JSONLoader &json);
 
  void clear() { IR::Vector<T>::clear(); declarations.clear(); }
- // Although this is not a const_iterator, it should NOT
+ // TODO: Although this is not a const_iterator, it should NOT
  // be used to modify the vector directly. I don't know
  // how to enforce this property, though.
  typedef typename Vector<T>::iterator iterator;
@@ -120,6 +120,16 @@ class IndexedVector : public Vector<T> {
  template <class... Args> void emplace_back(Args&&... args) {
  auto el = new T(std::forward<Args>(args)...);
  insert(el); }
+ bool removeByName(cstring name) {
+ for (auto it = begin(); it != end(); ++it) {
+ auto decl = (*it)->template to<IDeclaration>();
+ if (decl != nullptr && decl->getName() == name) {
+ erase(it);
+ return true;
+ }
+ }
+ return false;
+ }
  void push_back(T *a) { CHECK_NULL(a); Vector<T>::push_back(a); insertInMap(a); }
  void push_back(const T *a) { CHECK_NULL(a); Vector<T>::push_back(a); insertInMap(a); }
  void pop_back() {

midend/CMakeLists.txt

@@ -44,6 +44,7 @@ set (MIDEND_SRCS
 
 set (MIDEND_HDRS
  actionSynthesis.h
+ checkSize.h
  compileTimeOps.h
  complexComparison.h
  convertEnums.h

midend/actionSynthesis.h

@@ -17,6 +17,7 @@ limitations under the License.
 #ifndef _MIDEND_ACTIONSYNTHESIS_H_
 #define _MIDEND_ACTIONSYNTHESIS_H_
 
+#include "ir/ir.h"
 #include "frontends/common/resolveReferences/referenceMap.h"
 #include "frontends/p4/typeChecking/typeChecker.h"
 

midend/checkSize.h

@@ -0,0 +1,57 @@
+/*
+Copyright 2018 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.
+*/
+
+#ifndef _MIDEND_CHECKSIZE_H_
+#define _MIDEND_CHECKSIZE_H_
+
+#include "ir/ir.h"
+
+namespace P4 {
+
+/// Checks some possible misuses of the table size property
+class CheckTableSize : public Modifier {
+ public:
+ CheckTableSize() { setName("CheckTableSize"); }
+ bool preorder(IR::P4Table* table) override {
+ auto size = table->getSizeProperty();
+ if (size == nullptr)
+ return false;
+
+ bool deleteSize = false;
+ auto key = table->getKey();
+ if (key == nullptr) {
+ if (size->value != 1) {
+ ::warning("%1%: Size specified for table without keys", size);
+ deleteSize = true;
+ }
+ }
+ auto entries = table->properties->getProperty(IR::TableProperties::entriesPropertyName);
+ if (entries != nullptr && entries->isConstant) {
+ ::warning("%1%: Size specified for table with constant entries", size);
+ deleteSize = true;
+ }
+ if (deleteSize) {
+ auto props = IR::IndexedVector<IR::Property>(table->properties->properties);
+ props.removeByName(IR::TableProperties::sizePropertyName);
+ table->properties = new IR::TableProperties(table->properties->srcInfo, props);
+ }
+ return false;
+ }
+};
+
+} // namespace P4
+
+#endif /* _MIDEND_CHECKSIZE_H_ */

testdata/p4_16_samples/issue1478-bmv2.p4

@@ -0,0 +1,40 @@
+#include <core.p4>
+#include <v1model.p4>
+
+header hdr {}
+
+struct Headers {}
+
+struct Meta {}
+
+parser p(packet_in b, out Headers h, inout Meta m, inout standard_metadata_t sm) {
+ state start {
+ transition accept;
+ }
+}
+
+control vrfy(inout Headers h, inout Meta m) { apply {} }
+control update(inout Headers h, inout Meta m) { apply {} }
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) { apply {} }
+
+control deparser(packet_out b, in Headers h) { apply {} }
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+ table t1 {
+ size = 3;
+ actions = { NoAction; }
+ const default_action = NoAction;
+ }
+ table t2 {
+ key = { sm.ingress_port: exact; }
+ actions = { NoAction; }
+ const entries = {
+ 0 : NoAction();
+ }
+ size = 10;
+ }
+ apply { t1.apply(); t2.apply(); }
+}
+
+V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main;

testdata/p4_16_samples_outputs/issue1478-bmv2-first.p4

@@ -0,0 +1,69 @@
+#include <core.p4>
+#include <v1model.p4>
+
+header hdr {
+}
+
+struct Headers {
+}
+
+struct Meta {
+}
+
+parser p(packet_in b, out Headers h, inout Meta m, inout standard_metadata_t sm) {
+ state start {
+ transition accept;
+ }
+}
+
+control vrfy(inout Headers h, inout Meta m) {
+ apply {
+ }
+}
+
+control update(inout Headers h, inout Meta m) {
+ apply {
+ }
+}
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+ apply {
+ }
+}
+
+control deparser(packet_out b, in Headers h) {
+ apply {
+ }
+}
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+ table t1 {
+ size = 3;
+ actions = {
+ NoAction();
+ }
+ const default_action = NoAction();
+ }
+ table t2 {
+ key = {
+ sm.ingress_port: exact @name("sm.ingress_port") ;
+ }
+ actions = {
+ NoAction();
+ }
+ const entries = {
+ 9w0 : NoAction();
+
+ }
+
+ size = 10;
+ default_action = NoAction();
+ }
+ apply {
+ t1.apply();
+ t2.apply();
+ }
+}
+
+V1Switch<Headers, Meta>(p(), vrfy(), ingress(), egress(), update(), deparser()) main;
+

testdata/p4_16_samples_outputs/issue1478-bmv2-frontend.p4

@@ -0,0 +1,73 @@
+#include <core.p4>
+#include <v1model.p4>
+
+header hdr {
+}
+
+struct Headers {
+}
+
+struct Meta {
+}
+
+parser p(packet_in b, out Headers h, inout Meta m, inout standard_metadata_t sm) {
+ state start {
+ transition accept;
+ }
+}
+
+control vrfy(inout Headers h, inout Meta m) {
+ apply {
+ }
+}
+
+control update(inout Headers h, inout Meta m) {
+ apply {
+ }
+}
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+ apply {
+ }
+}
+
+control deparser(packet_out b, in Headers h) {
+ apply {
+ }
+}
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+ @name(".NoAction") action NoAction_0() {
+ }
+ @name(".NoAction") action NoAction_3() {
+ }
+ @name("ingress.t1") table t1 {
+ size = 3;
+ actions = {
+ NoAction_0();
+ }
+ const default_action = NoAction_0();
+ }
+ @name("ingress.t2") table t2 {
+ key = {
+ sm.ingress_port: exact @name("sm.ingress_port") ;
+ }
+ actions = {
+ NoAction_3();
+ }
+ const entries = {
+ 9w0 : NoAction_3();
+
+ }
+
+ size = 10;
+ default_action = NoAction_3();
+ }
+ apply {
+ t1.apply();
+ t2.apply();
+ }
+}
+
+V1Switch<Headers, Meta>(p(), vrfy(), ingress(), egress(), update(), deparser()) main;
+

testdata/p4_16_samples_outputs/issue1478-bmv2-midend.p4

@@ -0,0 +1,73 @@
+#include <core.p4>
+#include <v1model.p4>
+
+header hdr {
+}
+
+struct Headers {
+}
+
+struct Meta {
+}
+
+parser p(packet_in b, out Headers h, inout Meta m, inout standard_metadata_t sm) {
+ state start {
+ transition accept;
+ }
+}
+
+control vrfy(inout Headers h, inout Meta m) {
+ apply {
+ }
+}
+
+control update(inout Headers h, inout Meta m) {
+ apply {
+ }
+}
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+ apply {
+ }
+}
+
+control deparser(packet_out b, in Headers h) {
+ apply {
+ }
+}
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+ @name(".NoAction") action NoAction_0() {
+ }
+ @name(".NoAction") action NoAction_3() {
+ }
+ @name("ingress.t1") table t1 {
+ size = 3;
+ actions = {
+ NoAction_0();
+ }
+ const default_action = NoAction_0();
+ }
+ @name("ingress.t2") table t2 {
+ key = {
+ sm.ingress_port: exact @name("sm.ingress_port") ;
+ }
+ actions = {
+ NoAction_3();
+ }
+ const entries = {
+ 9w0 : NoAction_3();
+
+ }
+
+ size = 10;
+ default_action = NoAction_3();
+ }
+ apply {
+ t1.apply();
+ t2.apply();
+ }
+}
+
+V1Switch<Headers, Meta>(p(), vrfy(), ingress(), egress(), update(), deparser()) main;
+