Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bmv2] Unexpected modification of variable's value #242

Closed
vikramnathan opened this issue Jan 18, 2017 · 2 comments
Closed

[bmv2] Unexpected modification of variable's value #242

vikramnathan opened this issue Jan 18, 2017 · 2 comments
Assignees
Labels
fixed This topic is considered to be fixed.

Comments

@vikramnathan
Copy link

vikramnathan commented Jan 18, 2017

Relevant code with debug "print" statements:

register<bit<32>>(32w100) debug;
register<bit<32>>(32w1) reg;

action test() {
    Value val = {0};

    bool _pred = (val.field1 != 0);
    bit<32> inc = _pred ? 32w1 : 0;
    debug.write(0, _pred ? 32w1 : 32w0); // Print _pred
    debug.write(1, inc);                 // Print inc
    val.field1 = 32w1;
    debug.write(2, _pred ? 32w1 : 0);    // Print _pred again
    debug.write(3, inc);                 // Print inc again

    reg.write(32w0, val.field1);
}

Expected output reading debug from CLI:

debug[0] = 0
debug[1] = 0
debug[2] = 0
debug[3] = 0

Observed output:

debug[0] = 0
debug[1] = 0
debug[2] = 1
debug[3] = 1

This seems to suggest that updating val.field1 also updated inc and _pred, even though both were set several lines earlier. My hunch is that this is a compiler issue.

Source p4 and json:
test_bug4.json.txt
test_bug4.p4.txt

@mihaibudiu
Copy link
Contributor

@ChrisDodd: I think that this is a problem with the LocalCopyPropagation.
Code before LCP:

        val.field1 = 32w0;
        tmp_2 = val.field1 != 32w0;
        _pred = tmp_2;
        {
            bool cond;
            {
                bool pred;
                cond = _pred;
                pred = cond;
                tmp_3 = (pred ? 32w1 : tmp_3);
                cond = !cond;
                pred = cond;
                tmp_3 = (pred ? 32w0 : tmp_3);
            }
        }
        inc = tmp_3;
        {
            bool cond_0;
            {
                bool pred_0;
                cond_0 = _pred;
                pred_0 = cond_0;
                tmp_4 = (pred_0 ? 32w1 : tmp_4);
                cond_0 = !cond_0;
                pred_0 = cond_0;
                tmp_4 = (pred_0 ? 32w0 : tmp_4);
            }
        }
        debug.write(32w0, tmp_4);
        debug.write(32w1, inc);
        val.field1 = 32w1;
        debug.write(32w2, inc);
        reg.write(32w0, val.field1);

Code after LCP:

val.field1 = 32w0;
        tmp_2 = val.field1 != 32w0;
        _pred = val.field1 != 32w0;
        {
            {
                tmp_3 = (val.field1 != 32w0 ? 32w1 : tmp_3);
                tmp_3 = (!(val.field1 != 32w0) ? 32w0 : (val.field1 != 32w0 ? 32w1 : tmp_3));
            }
        }
        inc = (!(val.field1 != 32w0) ? 32w0 : (val.field1 != 32w0 ? 32w1 : tmp_3));
        {
            {
                tmp_4 = (val.field1 != 32w0 ? 32w1 : tmp_4);
                tmp_4 = (!(val.field1 != 32w0) ? 32w0 : (val.field1 != 32w0 ? 32w1 : tmp_4));
            }
        }
        debug.write(32w0, (!(val.field1 != 32w0) ? 32w0 : (val.field1 != 32w0 ? 32w1 : tmp_4)));
        debug.write(32w1, (!(val.field1 != 32w0) ? 32w0 : (val.field1 != 32w0 ? 32w1 : tmp_3)));
        val.field1 = 32w1;
        debug.write(32w2, (!(val.field1 != 32w0) ? 32w0 : (val.field1 != 32w0 ? 32w1 : tmp_3)));
        reg.write(32w0, val.field1);

The issue seems to be that val.field1 is assigned to, but this does not invalidate inc, which does depend on val.field1.

@ChrisDodd ChrisDodd self-assigned this Jan 20, 2017
mihaibudiu pushed a commit to mihaibudiu/p4c-clone that referenced this issue Jan 26, 2017
ChrisDodd pushed a commit that referenced this issue Jan 26, 2017
* Remove unused variable

* EBPF back-end improvements

* Reproduction case for issue #242

* Added missing return value
ChrisDodd pushed a commit to ChrisDodd/p4c that referenced this issue Feb 2, 2017
- propagate into rhs of assignment before left kill copies
- don't propagate values that are recursive
ChrisDodd pushed a commit to ChrisDodd/p4c that referenced this issue Feb 3, 2017
- propagate into rhs of assignment before left kill copies
- don't propagate values that are recursive
ChrisDodd pushed a commit to ChrisDodd/p4c that referenced this issue Feb 3, 2017
- propagate into rhs of assignment before left kill copies
- don't propagate values that are recursive
ChrisDodd pushed a commit to ChrisDodd/p4c that referenced this issue Feb 4, 2017
- propagate into rhs of assignment before left kill copies
- don't propagate values that are recursive
mihaibudiu pushed a commit to mihaibudiu/p4c-clone that referenced this issue Feb 6, 2017
ChrisDodd pushed a commit that referenced this issue Feb 6, 2017
@ChrisDodd ChrisDodd added the fixed This topic is considered to be fixed. label Feb 21, 2017
@ChrisDodd ChrisDodd assigned vikramnathan and unassigned ChrisDodd Feb 21, 2017
@vikramnathan
Copy link
Author

Verified as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed This topic is considered to be fixed.
Projects
None yet
Development

No branches or pull requests

3 participants