Some more predication issues

[fruffy]

I have collected some more issues with the predication pass that I have stumbled upon recently. Unfortunately, it is hard for me to distinguish whether they are unique or not. However, it looks like they have a similar source issue. @anasyrmia would you mind taking a look at this? I am really testing the limits of this pass.

The following program:

bool simple_check() {
    return true;
}

action assign(inout bit<16> eth_t, inout bit<48> target_addr, bool check_bool) {
    if (check_bool) {
        if (simple_check() && 0xDEAD != eth_t) {
            target_addr = 48w1;
        }
    }
}

is transformed to

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
    @name("ingress.tmp") bool tmp;
    @name("ingress.tmp_0") bool tmp_0;
    @name("ingress.hasReturned") bool hasReturned;
    @name("ingress.retval") bool retval;
    bit<16> eth_t;
    bit<48> target_addr;
    bool check_bool;
    @name(".assign") action assign() {
        eth_t = h.eth_hdr.eth_type;
        target_addr = h.eth_hdr.dst_addr;
        check_bool = true;
        {
            {
                {
                    bool cond;
                    cond = !tmp;
                    hasReturned = (check_bool ? true : hasReturned);
                    retval = (check_bool ? true : retval);
                    tmp = (check_bool ? retval : tmp);
                    tmp_0 = (check_bool ? (cond ? false : 16w0xdead != eth_t) : tmp_0);
                }
                {
                    target_addr = (check_bool ? (tmp_0 ? 48w1 : target_addr) : target_addr);
                }
            }
        }
        h.eth_hdr.eth_type = eth_t;
        h.eth_hdr.dst_addr = target_addr;
    }
    apply {
        assign();
    }
}

As far as I can tell, the problem is this assignment cond = !tmp;, where tmp is still undefined. So later expressions that depend on this value will be undefined. For example, (check_bool ? (cond ? false : 16w0xdead != eth_t) : tmp_0)
predication_issue_1.p4.txt
predication_issue_1.stf.txt

[fruffy]

Another similar issue is this one:

struct Headers {
    ethernet_t eth_hdr;
    IDX idx;
    H[2] h;
}

bit<8> bound(in bit<8> val, in bit<8> bound) {
    return (val < bound ? val : bound);
}

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
    action simple_action(bool check) {
        if (check) {
            bit<8> val = bound(h.idx.idx, 8w1);
            h.h[val].a = 8w1;
        }
    }
    apply {
        simple_action(true);
    }
}

which is transformed to

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
    @name("ingress.val") bit<8> val_0;
    @name("ingress.val_1") bit<8> val_1;
    @name("ingress.bound_0") bit<8> bound_0;
    @name("ingress.hasReturned") bool hasReturned;
    @name("ingress.retval") bit<8> retval;
    @name("ingress.tmp") bit<8> tmp;
    bool check_1;
    @name("ingress.simple_action") action simple_action() {
        check_1 = true;
        {
            {
                {
                    {
                        bool cond;
                        cond = val_1 < bound_0;
                        val_1 = (check_1 ? h.idx.idx : val_1);
                        bound_0 = (check_1 ? 8w1 : bound_0);
                        hasReturned = (check_1 ? false : hasReturned);
                        tmp = (check_1 ? (cond ? val_1 : tmp) : tmp);
                        tmp = (check_1 ? (cond ? val_1 : bound_0) : tmp);
                    }
                }
            }
            hasReturned = (check_1 ? true : hasReturned);
            retval = (check_1 ? tmp : retval);
            val_0 = (check_1 ? retval : val_0);
            h.h[val_0].a = (check_1 ? 8w1 : h.h[val_0].a);
        }
    }
    apply {
        simple_action();
    }
}

Here, cond = val_1 < bound_0 is assigned before either of these variables have a value, making the result undefined again.

predication_issue_2.p4.txt
predication_issue_2.stf.txt

[fruffy]

In the last example I have found the program

struct Headers {
    ethernet_t    eth_hdr;
    H[2] h;
}

struct Meta {}

bit<3> bound(in bit<3> val, in bit<3> bound) {
    return (val < bound ? val : bound);
}

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
    bool bool_val = true;
    action perform_action() {
        if (bool_val) {
            h.h[bound(3w0, 3w1)].a = 1;
        }
    }
    apply {
        perform_action();
    }
}

is transformed to

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
    @name("ingress.bool_val") bool bool_val_0;
    @name("ingress.tmp_0") bit<3> tmp;
    @name("ingress.tmp_1") bit<3> tmp_0;
    @name("ingress.val_0") bit<3> val_0;
    @name("ingress.bound_0") bit<3> bound_0;
    @name("ingress.hasReturned") bool hasReturned;
    @name("ingress.retval") bit<3> retval;
    @name("ingress.tmp") bit<3> tmp_1;
    @name("ingress.perform_action") action perform_action() {
        {
            {
                {
                    {
                        bool cond;
                        cond = val_0 < bound_0;
                        val_0 = (bool_val_0 ? 3w0 : val_0);
                        bound_0 = (bool_val_0 ? 3w1 : bound_0);
                        hasReturned = (bool_val_0 ? false : hasReturned);
                        tmp_1 = (bool_val_0 ? (cond ? val_0 : tmp_1) : tmp_1);
                        tmp_1 = (bool_val_0 ? (cond ? val_0 : bound_0) : tmp_1);
                    }
                }
            }
            hasReturned = (bool_val_0 ? true : hasReturned);
            retval = (bool_val_0 ? tmp_1 : retval);
            tmp = (bool_val_0 ? retval : tmp);
            tmp_0 = (bool_val_0 ? tmp : tmp_0);
            h.h[tmp_0].a = (bool_val_0 ? 8w1 : h.h[tmp_0].a);
        }
    }
    apply {
        bool_val_0 = true;
        perform_action();
    }
}

Again, the condition is assigned before the variables have been evaluated, leading to undefined behavior.

predication_issue_3.stf.txt
predication_issue_3.p4.txt

[mihaibudiu]

@matijasyrmia @anasyrmia if you do not have time to address these issues I am considering reverting some of your improvements to the predication pass, since it is too complex for me to debug.

[anasyrmia]

Hello @mbudiu-vmw, my apologies for the delayed response.
One of our colleagues will work on fixing this issue.

[filipsyrmia]

Hello @mbudiu-vmw, I've spoken with @anasyrmia and got familiar with the issue.
I would like to work on fixing these problems if that's alright with you. Also I will try to comment out the code properly, making it easier for potential further debugging.

[mihaibudiu]

How could I object to such suggestion?