Fixing Compiler bug

backends/p4tools/modules/smith/scripts/compilation-test.sh

@@ -12,7 +12,6 @@ KNOWN_BUGS=(
     "Cannot evaluate initializer for constant"
     "Null expr"
     "error: retval_1: declaration not found"  # V1model failure.
-    "Compiler Bug: Cannot find declaration for inlinedRetval" # See #5549
     "At this point in the compilation typechecking should not infer new types anymore" # See #5560
 )
 

backends/tofino/bf-p4c/midend/remove_action_params.h

@@ -35,8 +35,9 @@ namespace BFN {
  */
 class DoRemoveActionParametersTofino : public P4::DoRemoveActionParameters {
  public:
-    explicit DoRemoveActionParametersTofino(P4::ActionInvocation *inv)
-        : P4::DoRemoveActionParameters(inv) {}
+    DoRemoveActionParametersTofino(P4::ActionInvocation *inv, P4::TypeMap *typeMap,
+                                   MinimalNameGenerator *nameGen)
+        : P4::DoRemoveActionParameters(inv, typeMap, nameGen) {}
 
     /**
      * Check if there is a mark_to_drop action and don't replace parameters.
@@ -186,8 +187,10 @@ class RemoveActionParameters : public PassManager {
         passes.emplace_back(new P4::MoveDeclarations());
         if (!tc) tc = new P4::TypeChecking(refMap, typeMap);
         passes.emplace_back(tc);
+        auto nameGen = new MinimalNameGenerator();
         passes.emplace_back(new P4::FindActionParameters(typeMap, ai));
-        passes.emplace_back(new DoRemoveActionParametersTofino(ai));
+        passes.emplace_back(nameGen);
+        passes.emplace_back(new DoRemoveActionParametersTofino(ai, typeMap, nameGen));
         passes.emplace_back(new P4::ClearTypeMap(typeMap));
     }
 };

frontends/p4/removeParameters.cpp

@@ -19,12 +19,47 @@ limitations under the License.
 #include "frontends/common/resolveReferences/resolveReferences.h"
 #include "frontends/p4/methodInstance.h"
 #include "frontends/p4/moveDeclarations.h"
+#include "frontends/p4/sideEffects.h"
 #include "frontends/p4/tableApply.h"
 #include "frontends/p4/typeChecking/typeChecker.h"
 
 namespace P4 {
 
 namespace {
+
+// Extract side-effecting array index expressions into temporaries
+// to ensure they are evaluated exactly once for inout copy-in/copy-out.
+class ExtractArrayIndices : public Transform {
+    TypeMap *typeMap;
+    MinimalNameGenerator &nameGen;
+    IR::IndexedVector<IR::Declaration> &tempDecls;
+    IR::IndexedVector<IR::StatOrDecl> &tempAssigns;
+
+ public:
+    ExtractArrayIndices(TypeMap *typeMap, MinimalNameGenerator &nameGen,
+                        IR::IndexedVector<IR::Declaration> &tempDecls,
+                        IR::IndexedVector<IR::StatOrDecl> &tempAssigns)
+        : typeMap(typeMap), nameGen(nameGen), tempDecls(tempDecls), tempAssigns(tempAssigns) {
+        setName("ExtractArrayIndices");
+    }
+    const IR::Node *preorder(IR::ArrayIndex *aindex) override {
+        visit(aindex->left);
+        if (SideEffects::check(aindex->right, called_by, typeMap)) {
+            if (auto indexType = typeMap->getType(aindex->right, true)) {
+                auto tmp = nameGen.newName("tmp_idx");
+                auto decl = new IR::Declaration_Variable(aindex->right->srcInfo, tmp, indexType);
+                tempDecls.push_back(decl);
+                auto assign = new IR::AssignmentStatement(
+                    aindex->right->srcInfo, new IR::PathExpression(tmp), aindex->right);
+                tempAssigns.push_back(assign);
+                aindex->right = new IR::PathExpression(aindex->right->srcInfo, new IR::Path(tmp));
+            }
+        }
+        prune();
+        return aindex;
+    }
+};
+
 // Remove arguments from any embedded MethodCallExpression
 class RemoveMethodCallArguments : public Transform {
     int argumentsToRemove;  // -1 => all
@@ -133,16 +168,34 @@ const IR::Node *DoRemoveActionParameters::postorder(IR::P4Action *action) {
                 continue;
             }
 
-            if (p->direction == IR::Direction::In || p->direction == IR::Direction::InOut ||
-                p->direction == IR::Direction::None) {
+            if (p->direction == IR::Direction::InOut) {
+                // For inout parameters, extract side-effecting array indices
+                // into temporaries to evaluate them once and avoid DAG sharing.
+                IR::IndexedVector<IR::Declaration> tempDecls;
+                IR::IndexedVector<IR::StatOrDecl> tempAssigns;
+                ExtractArrayIndices eai(typeMap, *nameGen, tempDecls, tempAssigns);
+                eai.setCalledBy(this);
+                auto argExpr = arg->expression->apply(eai)->to<IR::Expression>();
+                for (auto *d : tempDecls) result->push_back(d);
+                body.append(tempAssigns);
+
+                auto left = new IR::PathExpression(p->name);
+                auto assign = new IR::AssignmentStatement(arg->srcInfo, left, argExpr);
+                body.push_back(assign);
+
+                auto right = new IR::PathExpression(p->name);
+                auto assign2 = new IR::AssignmentStatement(arg->srcInfo, argExpr->clone(), right);
+                postamble.push_back(assign2);
+            } else if (p->direction == IR::Direction::In || p->direction == IR::Direction::None) {
                 auto left = new IR::PathExpression(p->name);
                 auto assign = new IR::AssignmentStatement(arg->srcInfo, left, arg->expression);
                 body.push_back(assign);
-            }
-            if (p->direction == IR::Direction::Out || p->direction == IR::Direction::InOut) {
+            } else if (p->direction == IR::Direction::Out) {
                 auto right = new IR::PathExpression(p->name);
                 auto assign = new IR::AssignmentStatement(arg->srcInfo, arg->expression, right);
                 postamble.push_back(assign);
+            } else {
+                BUG("unhandled direction %1%", directionToString(p->direction));
             }
         }
     }
@@ -197,8 +250,10 @@ RemoveActionParameters::RemoveActionParameters(TypeMap *typeMap, TypeChecking *t
     passes.emplace_back(new MoveDeclarations());
     if (!typeChecking) typeChecking = new TypeChecking(nullptr, typeMap);
     passes.emplace_back(typeChecking);
+    auto nameGen = new MinimalNameGenerator();
     passes.emplace_back(new FindActionParameters(typeMap, ai));
-    passes.emplace_back(new DoRemoveActionParameters(ai));
+    passes.emplace_back(nameGen);
+    passes.emplace_back(new DoRemoveActionParameters(ai, typeMap, nameGen));
     passes.emplace_back(new ClearTypeMap(typeMap));
 }
 

frontends/p4/removeParameters.h

@@ -114,10 +114,16 @@ class FindActionParameters : public Inspector, public ResolutionContext {
  */
 class DoRemoveActionParameters : public Transform {
     ActionInvocation *invocations;
+    TypeMap *typeMap;
+    MinimalNameGenerator *nameGen;
 
  public:
-    explicit DoRemoveActionParameters(ActionInvocation *invocations) : invocations(invocations) {
+    DoRemoveActionParameters(ActionInvocation *invocations, TypeMap *typeMap,
+                             MinimalNameGenerator *nameGen)
+        : invocations(invocations), typeMap(typeMap), nameGen(nameGen) {
         CHECK_NULL(invocations);
+        CHECK_NULL(typeMap);
+        CHECK_NULL(nameGen);
         setName("DoRemoveActionParameters");
     }
 

testdata/p4_16_samples/inline-function-to-array-index-chained.p4

@@ -0,0 +1,38 @@
+#include <core.p4>
+
+bit<3> max(in bit<3> val, in bit<3> bound) {
+    return val < bound ? val : bound;
+}
+
+header inner_t {
+    bit<8> f2;
+}
+
+header outer_t {
+    bit<8> f1;
+}
+
+struct mid_t {
+    inner_t[1] inner_arr;
+}
+
+struct Headers {
+    outer_t[1] outer_arr;
+    mid_t[1] mid_arr;
+}
+
+control proto(inout Headers h);
+package top(proto _c);
+
+// Test: expr[i1].f1[i2].f2 — chained array indices with function calls and field accesses
+control ingress(inout Headers h) {
+    action a(inout bit<8> val) {
+        val = 8w1;
+    }
+    table t {
+        actions = { a(h.mid_arr[max(3w0, 3w0)].inner_arr[max(3w0, 3w0)].f2); }
+    }
+    apply { t.apply(); }
+}
+
+top(ingress()) main;

testdata/p4_16_samples/inline-function-to-array-index.p4

@@ -0,0 +1,24 @@
+#include <core.p4>
+
+bit<3> max(in bit<3> val, in bit<3> bound) {
+    return val < bound ? val : bound;
+}
+
+header h_t {}
+
+struct Headers {
+    h_t[1] hdr_arr;
+}
+
+control proto(inout Headers h);
+package top(proto _c);
+
+control ingress(inout Headers h) {
+    action a(inout h_t hdr) {}
+    table t {
+        actions = { a(h.hdr_arr[max(3w0, 3w0)]); }
+    }
+    apply { t.apply(); }
+}
+
+top(ingress()) main;

testdata/p4_16_samples_outputs/inline-function-to-array-index-chained-first.p4

@@ -0,0 +1,41 @@
+#include <core.p4>
+
+bit<3> max(in bit<3> val, in bit<3> bound) {
+    return (val < bound ? val : bound);
+}
+header inner_t {
+    bit<8> f2;
+}
+
+header outer_t {
+    bit<8> f1;
+}
+
+struct mid_t {
+    inner_t[1] inner_arr;
+}
+
+struct Headers {
+    outer_t[1] outer_arr;
+    mid_t[1]   mid_arr;
+}
+
+control proto(inout Headers h);
+package top(proto _c);
+control ingress(inout Headers h) {
+    action a(inout bit<8> val) {
+        val = 8w1;
+    }
+    table t {
+        actions = {
+            a(h.mid_arr[max(3w0, 3w0)].inner_arr[max(3w0, 3w0)].f2);
+            @defaultonly NoAction();
+        }
+        default_action = NoAction();
+    }
+    apply {
+        t.apply();
+    }
+}
+
+top(ingress()) main;

testdata/p4_16_samples_outputs/inline-function-to-array-index-chained-frontend.p4

@@ -0,0 +1,74 @@
+#include <core.p4>
+
+header inner_t {
+    bit<8> f2;
+}
+
+header outer_t {
+    bit<8> f1;
+}
+
+struct mid_t {
+    inner_t[1] inner_arr;
+}
+
+struct Headers {
+    outer_t[1] outer_arr;
+    mid_t[1]   mid_arr;
+}
+
+control proto(inout Headers h);
+package top(proto _c);
+control ingress(inout Headers h) {
+    @name("ingress.val") bit<8> val;
+    @name("ingress.tmp_idx") bit<3> tmp_idx_1;
+    @name("ingress.tmp_idx_0") bit<3> tmp_idx_2;
+    @name("ingress.val_0") bit<3> val_3;
+    @name("ingress.bound_0") bit<3> bound;
+    @name("ingress.retval") bit<3> retval;
+    @name("ingress.tmp") bit<3> tmp;
+    @name("ingress.inlinedRetval") bit<3> inlinedRetval_1;
+    @name("ingress.val_2") bit<3> val_4;
+    @name("ingress.bound_1") bit<3> bound_2;
+    @name("ingress.retval") bit<3> retval_1;
+    @name("ingress.tmp") bit<3> tmp_1;
+    @name("ingress.inlinedRetval_0") bit<3> inlinedRetval_2;
+    @noWarn("unused") @name(".NoAction") action NoAction_1() {
+    }
+    @name("ingress.a") action a() {
+        val_3 = 3w0;
+        bound = 3w0;
+        if (val_3 < bound) {
+            tmp = val_3;
+        } else {
+            tmp = bound;
+        }
+        retval = tmp;
+        inlinedRetval_1 = retval;
+        tmp_idx_1 = inlinedRetval_1;
+        val_4 = 3w0;
+        bound_2 = 3w0;
+        if (val_4 < bound_2) {
+            tmp_1 = val_4;
+        } else {
+            tmp_1 = bound_2;
+        }
+        retval_1 = tmp_1;
+        inlinedRetval_2 = retval_1;
+        tmp_idx_2 = inlinedRetval_2;
+        val = 8w1;
+        h.mid_arr[tmp_idx_1].inner_arr[tmp_idx_2].f2 = val;
+    }
+    @name("ingress.t") table t_0 {
+        actions = {
+            a();
+            @defaultonly NoAction_1();
+        }
+        default_action = NoAction_1();
+    }
+    apply {
+        t_0.apply();
+    }
+}
+
+top(ingress()) main;

testdata/p4_16_samples_outputs/inline-function-to-array-index-chained-midend.p4

@@ -0,0 +1,40 @@
+#include <core.p4>
+
+header inner_t {
+    bit<8> f2;
+}
+
+header outer_t {
+    bit<8> f1;
+}
+
+struct mid_t {
+    inner_t[1] inner_arr;
+}
+
+struct Headers {
+    outer_t[1] outer_arr;
+    mid_t[1]   mid_arr;
+}
+
+control proto(inout Headers h);
+package top(proto _c);
+control ingress(inout Headers h) {
+    @noWarn("unused") @name(".NoAction") action NoAction_1() {
+    }
+    @name("ingress.a") action a() {
+        h.mid_arr[3w0].inner_arr[3w0].f2 = 8w1;
+    }
+    @name("ingress.t") table t_0 {
+        actions = {
+            a();
+            @defaultonly NoAction_1();
+        }
+        default_action = NoAction_1();
+    }
+    apply {
+        t_0.apply();
+    }
+}
+
+top(ingress()) main;