Fixing bug in HSIndexSimplifier path (#3382)

* fixing bug #issue3374

Author: VolodymyrPeschanenkoIntel

midend/hsIndexSimplify.cpp

@@ -51,7 +51,7 @@ const IR::Node* HSIndexTransform::postorder(IR::ArrayIndex* curArrayIndex) {
     return curArrayIndex;
 }
 
-IR::Node* HSIndexSimplifier::eliminateArrayIndexes(HSIndexFinder& aiFinder,
+IR::Node* HSIndexContretizer::eliminateArrayIndexes(HSIndexFinder& aiFinder,
                                                    IR::Statement* statement,
                                                    const IR::Expression* expr) {
     if (aiFinder.arrayIndex == nullptr) {
@@ -94,7 +94,7 @@ IR::Node* HSIndexSimplifier::eliminateArrayIndexes(HSIndexFinder& aiFinder,
     }
     if (expr != nullptr && locals != nullptr) {
         // Add case for write out of bound.
-        cstring typeString = expr->type->node_type_name();
+        cstring typeString = expr->type->toString();
         const IR::PathExpression* pathExpr = nullptr;
         if (generatedVariables->count(typeString) == 0) {
             // Add assigment of undefined header.
@@ -120,7 +120,7 @@ IR::Node* HSIndexSimplifier::eliminateArrayIndexes(HSIndexFinder& aiFinder,
     return new IR::BlockStatement(newComponents);
 }
 
-IR::Node* HSIndexSimplifier::preorder(IR::AssignmentStatement* assignmentStatement) {
+IR::Node* HSIndexContretizer::preorder(IR::AssignmentStatement* assignmentStatement) {
     HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
     assignmentStatement->left->apply(aiFinder);
     if (aiFinder.arrayIndex == nullptr) {
@@ -157,12 +157,13 @@ class IsNonConstantArrayIndex : public KeyIsSimple, public Inspector {
     }
 };
 
-IR::Node* HSIndexSimplifier::preorder(IR::P4Control* control) {
+IR::Node* HSIndexContretizer::preorder(IR::P4Control* control) {
     DoSimplifyKey keySimplifier(refMap, typeMap, new IsNonConstantArrayIndex());
     const auto* controlKeySimplified = control->apply(keySimplifier)->to<IR::P4Control>();
     auto* newControl = controlKeySimplified->clone();
     IR::IndexedVector<IR::Declaration> newControlLocals;
-    HSIndexSimplifier hsSimplifier(refMap, typeMap, &newControlLocals, generatedVariables);
+    GeneratedVariablesMap blockGeneratedVariables;
+    HSIndexContretizer hsSimplifier(refMap, typeMap, &newControlLocals, &blockGeneratedVariables);
     newControl->body = newControl->body->apply(hsSimplifier)->to<IR::BlockStatement>();
     for (auto* declaration : controlKeySimplified->controlLocals) {
         if (declaration->is<IR::P4Action>()) {
@@ -175,19 +176,18 @@ IR::Node* HSIndexSimplifier::preorder(IR::P4Control* control) {
     return newControl;
 }
 
-IR::Node* HSIndexSimplifier::preorder(IR::P4Parser* parser) {
+IR::Node* HSIndexContretizer::preorder(IR::P4Parser* parser) {
     prune();
     return parser;
 }
 
-IR::Node* HSIndexSimplifier::preorder(IR::BlockStatement* blockStatement) {
-    GeneratedVariablesMap blockGeneratedVariables;
-    HSIndexFinder aiFinder(locals, refMap, typeMap, &blockGeneratedVariables);
+IR::Node* HSIndexContretizer::preorder(IR::BlockStatement* blockStatement) {
+    HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
     blockStatement->apply(aiFinder);
     if (aiFinder.arrayIndex == nullptr) {
         return blockStatement;
     }
-    HSIndexSimplifier hsSimplifier(refMap, typeMap, locals, &blockGeneratedVariables);
+    HSIndexContretizer hsSimplifier(refMap, typeMap, locals, generatedVariables);
     auto* newBlock = blockStatement->clone();
     IR::IndexedVector<IR::StatOrDecl> newComponents;
     for (auto& component : blockStatement->components) {
@@ -204,21 +204,21 @@ IR::Node* HSIndexSimplifier::preorder(IR::BlockStatement* blockStatement) {
     return newBlock;
 }
 
-IR::Node* HSIndexSimplifier::preorder(IR::IfStatement* ifStatement) {
+IR::Node* HSIndexContretizer::preorder(IR::IfStatement* ifStatement) {
     HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
     ifStatement->condition->apply(aiFinder);
     return eliminateArrayIndexes(aiFinder, ifStatement, nullptr);
 }
 
-IR::Node* HSIndexSimplifier::preorder(IR::MethodCallStatement* methodCallStatement) {
+IR::Node* HSIndexContretizer::preorder(IR::MethodCallStatement* methodCallStatement) {
     HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
     methodCallStatement->apply(aiFinder);
     // Here we mean that in/out parameter will be replaced by correspondent assignments.
     // In this case no need to consider assignment to undefined value.
     return eliminateArrayIndexes(aiFinder, methodCallStatement, nullptr);
 }
 
-IR::Node* HSIndexSimplifier::preorder(IR::SwitchStatement* switchStatement) {
+IR::Node* HSIndexContretizer::preorder(IR::SwitchStatement* switchStatement) {
     HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
     switchStatement->expression->apply(aiFinder);
     return eliminateArrayIndexes(aiFinder, switchStatement, nullptr);

midend/hsIndexSimplify.h

@@ -15,7 +15,7 @@ typedef std::map<cstring, const IR::PathExpression*> GeneratedVariablesMap;
 /// adds it to the corresponded local definitions.
 class HSIndexFinder : public Inspector {
     friend class HSIndexTransform;
-    friend class HSIndexSimplifier;
+    friend class HSIndexContretizer;
     IR::IndexedVector<IR::Declaration>* locals;
     ReferenceMap* refMap;
     TypeMap* typeMap;
@@ -42,7 +42,7 @@ class HSIndexFinder : public Inspector {
 
 /// This class substitutes index of a header stack in all occurence of found header stack.
 class HSIndexTransform : public Transform {
-    friend class HSIndexSimplifier;
+    friend class HSIndexContretizer;
     int index;
     HSIndexFinder& hsIndexFinder;
 
@@ -66,14 +66,14 @@ class HSIndexTransform : public Transform {
 /// hdivr0 = hdr.i;
 /// if (hdivr0 == 0) { hdr.h[0] = 1;}
 /// else if (hdivr0 == 1){hdr.h[1] = 1;}
-class HSIndexSimplifier : public Transform {
+class HSIndexContretizer : public Transform {
     ReferenceMap* refMap;
     TypeMap* typeMap;
     IR::IndexedVector<IR::Declaration>* locals;
     GeneratedVariablesMap* generatedVariables;
 
  public:
-    HSIndexSimplifier(ReferenceMap* refMap, TypeMap* typeMap,
+    HSIndexContretizer(ReferenceMap* refMap, TypeMap* typeMap,
                       IR::IndexedVector<IR::Declaration>* locals = nullptr,
                       GeneratedVariablesMap* generatedVariables = nullptr)
         : refMap(refMap), typeMap(typeMap), locals(locals), generatedVariables(generatedVariables) {
@@ -92,6 +92,17 @@ class HSIndexSimplifier : public Transform {
                                     const IR::Expression* expr);
 };
 
+class HSIndexSimplifier : public PassManager {
+ public:
+    HSIndexSimplifier(ReferenceMap* refMap, TypeMap* typeMap) {
+        // remove block statements
+        passes.push_back(new TypeChecking(refMap, typeMap, true));
+        passes.push_back(new HSIndexContretizer(refMap, typeMap));
+        setName("HSIndexSimplifier");
+    }
+};
+
+
 }  // namespace P4
 
 #endif /*  _MIDEND_HSINDEXSIMPLIFY_H_ */

testdata/p4_16_samples/issue3374.p4

@@ -0,0 +1,166 @@
+/*
+Copyright 2022 Intel Corporation
+
+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.
+*/
+
+#include <core.p4>
+#include <v1model.p4>
+
+#define MAX_LAYERS 2
+typedef bit<48>  EthernetAddress;
+
+enum bit<16> ether_type_t {
+    TPID = 0x8100,
+    IPV4 = 0x0800,
+    IPV6 = 0x86DD
+}
+
+header ethernet_t {
+    EthernetAddress dstAddr;
+    EthernetAddress srcAddr;
+    bit<16>         etherType;
+}
+
+header vlan_tag_h {
+    bit<3>        pcp;
+    bit<1>        cfi;
+    bit<12>       vid;
+    ether_type_t  ether_type;
+}
+
+struct headers_t {
+    ethernet_t ethernet;
+    vlan_tag_h[MAX_LAYERS]      vlan_tag;
+}
+
+struct main_metadata_t {
+    bit<2> depth;
+    bit<16> ethType;
+}
+
+parser parserImpl(
+    packet_in pkt,
+    out headers_t hdrs,
+    inout main_metadata_t meta,
+    inout standard_metadata_t stdmeta)
+{
+    state start {
+        meta.depth = MAX_LAYERS - 1;
+        pkt.extract(hdrs.ethernet);
+        transition select(hdrs.ethernet.etherType) {
+            ether_type_t.TPID :  parse_vlan_tag;
+            default: accept;
+        }
+    }
+    state parse_vlan_tag {
+        pkt.extract(hdrs.vlan_tag.next);
+        meta.depth = meta.depth - 1;
+        transition select(hdrs.vlan_tag.last.ether_type) {
+            ether_type_t.TPID :  parse_vlan_tag;
+            default: accept;
+        }
+    }
+}
+
+control verifyChecksum(
+    inout headers_t hdr,
+    inout main_metadata_t meta)
+{
+    apply { }
+}
+
+control ingressImpl(
+    inout headers_t hdrs,
+    inout main_metadata_t meta,
+    inout standard_metadata_t stdmeta)
+{
+    action drop_packet() {
+        mark_to_drop(stdmeta);
+    }
+    action execute() {
+        meta.ethType = hdrs.vlan_tag[meta.depth - 1].ether_type;
+        hdrs.vlan_tag[meta.depth - 1].ether_type = (ether_type_t)16w2;
+        hdrs.vlan_tag[meta.depth].vid = (bit<12>)hdrs.vlan_tag[meta.depth].cfi;
+        hdrs.vlan_tag[meta.depth].vid = hdrs.vlan_tag[meta.depth-1].vid;
+ //       hdrs.vlan_tag[meta.depth].vid = hdrs.vlan_tag[1].vid;
+    }
+    action execute_1() {
+         drop_packet();
+    }
+		
+    table stub {
+        key = {
+              hdrs.vlan_tag[meta.depth].vid : exact;
+        }
+
+        actions = {
+		execute;
+        }
+	const default_action = execute;
+        size=1000000;
+    }
+
+    table stub1 {
+        key = {
+              hdrs.ethernet.etherType : exact;
+        }
+
+        actions = {
+		execute_1;
+        }
+	const default_action = execute_1;
+        size=1000000;
+    }
+    apply {
+        switch (hdrs.vlan_tag[meta.depth].vid) {
+//        switch (hdrs.vlan_tag[0].vid) {
+            12w1: { stub.apply();}
+            12w2: {
+               if (hdrs.vlan_tag[meta.depth].ether_type == hdrs.ethernet.etherType)
+                   stub1.apply();
+            }
+        }
+    }
+}
+
+control egressImpl(
+    inout headers_t hdr,
+    inout main_metadata_t meta,
+    inout standard_metadata_t stdmeta)
+{
+    apply { }
+}
+
+control updateChecksum(
+    inout headers_t hdr,
+    inout main_metadata_t meta)
+{
+    apply { }
+}
+
+control deparserImpl(
+    packet_out pkt,
+    in headers_t hdr)
+{
+    apply {
+        pkt.emit(hdr.ethernet);
+    }
+}
+
+V1Switch(parserImpl(),
+         verifyChecksum(),
+         ingressImpl(),
+         egressImpl(),
+         updateChecksum(),
+         deparserImpl()) main;

testdata/p4_16_samples_outputs/control-hs-index-test6-midend.p4

@@ -39,7 +39,6 @@ parser p(packet_in pkt, out headers hdr, inout Meta m, inout standard_metadata_t
 
 control ingress(inout headers h, inout Meta m, inout standard_metadata_t sm) {
     bit<32> hsiVar;
-    bit<32> hsiVar_0;
     bit<32> hsVar;
     @noWarn("unused") @name(".NoAction") action NoAction_1() {
     }
@@ -66,7 +65,7 @@ control ingress(inout headers h, inout Meta m, inout standard_metadata_t sm) {
         key_0 = hsVar;
     }
     @hidden action controlhsindextest6l48_2() {
-        hsiVar_0 = h.i.index;
+        hsiVar = h.i.index;
     }
     @hidden table tbl_controlhsindextest6l48 {
         actions = {
@@ -94,11 +93,11 @@ control ingress(inout headers h, inout Meta m, inout standard_metadata_t sm) {
     }
     apply {
         tbl_controlhsindextest6l48.apply();
-        if (hsiVar_0 == 32w0) {
+        if (hsiVar == 32w0) {
             tbl_controlhsindextest6l48_0.apply();
-        } else if (hsiVar_0 == 32w1) {
+        } else if (hsiVar == 32w1) {
             tbl_controlhsindextest6l48_1.apply();
-        } else if (hsiVar_0 >= 32w1) {
+        } else if (hsiVar >= 32w1) {
             tbl_controlhsindextest6l48_2.apply();
         }
         t_0.apply();