Compiler Bug: Overwriting definitions at <MethodCallExpression>(22038)//<MethodCallExpression>(22017)//<BoolLiteral>(1718)

[kfcripps]

Compiling the following P4 code:

extern void __e(in bit<28> arg);
extern void __e2(in bit<28> arg);

control C() {
    action bar(bool a, bool b) {
        bit<28> x; bit<28> y;
        if (a) {
            if (b) {
                __e(x);
            }
        } else {
            if (b) {
                __e2(y);
            }
        }
    }

    action foo() {
        bar(true, false);
    }

    table t {
        actions = { foo; }
        default_action = foo;
    }

    apply {
        t.apply();
    }
}

control proto();
package top(proto p);

top(C()) main;

results in:

In file: ... frontends/p4/def_use.h:456
Compiler Bug: Overwriting definitions at <MethodCallExpression>(22038)//<MethodCallExpression>(22017)//<BoolLiteral>(1718)

This bug has been introduced somewhere in between 812722f and 0685e1a.

[kfcripps]

This was also introduced by e60cb8a. :)

[kfcripps]

Actions inlining transforms foo() to something like this:

    @name("foo") action foo_0() {
        {
            @name("x") bit<28> x_0;
            @name("y") bit<28> y_0;
            if (true) {
                if (false) {
                    __e(x_0);
                }
            } else if (false) {
                __e2(y_0);
            }
        }
    }

The problem is similar to #4500.

The condition for both "if (false)" IR::IfStatements point to the same IR::BoolLiteral, which is not expected by SimplifyDefUse.

I was able to fix this with the following change to SubstituteParameters::postorder(IR::PathExpression *expr):

...
        auto value = subst->lookup(param)->expression->clone();
...

[kfcripps]

I was able to fix this with the following change to SubstituteParameters::postorder(IR::PathExpression *expr):

I am not sure if it would better to perform the cloning here or in ActionsInliner::preorder(IR::MethodCallStatement *statement) instead.

[kfcripps]

Here is another case, which hits the same Compiler bug, but is not fixed by my change described above:

extern void __e(in bit<28> arg);
extern void __e2(in bit<28> arg);

control C() {
    action bar(bool a, bool b) {
        bit<28> x; bit<28> y;
        if (a) {
            if (b) {
                __e(x);
            }
        } else {
            if (b) {
                __e2(y);
            }
        }
    }

    action baz() {
        bar(true, false);
    }

    action foo() {
        baz();
        baz();
    }

    table t {
        actions = { foo; }
        default_action = foo;
    }

    apply {
        t.apply();
    }
}

control proto();
package top(proto p);

top(C()) main;

[kfcripps]

or by inserting a pass that converts DAGs into trees before the def-use analysis.

This is starting to sound like the best solution..