SimplifyDefUse incorrectly removes assignment in actions with slices as arguments

[fruffy]

Because of copy-out semantics, the compiler removes assignments to global variables if they are also passed as an argument.
For example:

control ingress(inout Headers h) {
    action do_action(inout bit<8> val) {
        h.h.a = 8w7;
    }

    apply {
        do_action(h.h.a);
    }
}

turns into

control ingress(inout Headers h) {
    @name("ingress.do_action") action do_action() {
    }
    apply {
        do_action();
    }
}

However, when slices are passed as argument and the whole variable is modified inside the action, the statement is still removed.

control ingress(inout Headers h) {
    action do_action(inout bit<7> val) {
        h.h.a[0:0] = 0;
    }
    apply {
        do_action(h.h.a[7:1]);
    }
}

turns into

control ingress(inout Headers h) {
    bit<7> val;
    @name("ingress.do_action") action do_action() {
        val = h.h.a[7:1];
        h.h.a[7:1] = val;
    }
    apply {
        do_action();
    }
}

and then ultimately all statements are removed. This incorrectly removes the h.h.a[0:0] = 0; assignment.

I attached a sample program and the corresponding out I would have expected. Feel free to correct me if I am making the wrong assumptions here.

slice_arg.stf.txt
slice_arg.txt

[fruffy]

Also I just noticed that in the first example the statement is removed even though the parameter is passed as read only. Does copy-in copy-out also apply to in parameters?

Just realized I was misreading my own code. The example does not remove the assignment when passing an in-value. This works as intended.

[jafingerhut]

Your first example looks like the compiler is doing the wrong thing to me. Removing any assignment to an out or inout parameter of a top level control seems wrong. Without additional special information in the back end that such an out or inout parameter of a top level control is not used by the architecture at all, removing that assignment changes the behavior of the system.

[jafingerhut]

I do not understand your comment that "Also I just noticed that in the first example the statement is removed even though the parameter is passed as read only." Your do_action action has one parameter with direction inout. What do you mean by "passed as read only"?

copy-in copy-out behavior applies to all controls, parsers, actions, functions, extern functions, and extern methods, throughout the language. The only things that are not copied-in or copied-out are directionless parameters, that I can think of right now, if they are extern objects, which are passed "by reference", sort of.

[fruffy]

Just updated my comment, realized I was misreading my own code.

[jafingerhut]

Sorry, I think I was misreading your initial code example.

control ingress(inout Headers h) {
    action do_action(inout bit<8> val) {
        h.h.a = 8w7;
    }

    apply {
        do_action(h.h.a);
    }
}

In that code, at the time of the call to action do_action, h.h.a is copied in to parameter val.

Then you modify h.h.a inside the action.

As do_action is returning, it copies the final value of val back to the caller's parameters, which is h.h.a, overwriting the assignment to h.h.a that was done inside of the action, back to its original value, because val has not been changed.

So optimizing away the assignment to h.h.a inside of the do_action call does look correct to me. It isn't necessary for the compiler to do, but it is correct.

[mihaibudiu]

The problem is that the def_use pass assumes that the slice assignment h.h.a[0] = 0 overwrites the entire h.h.a field, but that is obviously not true. The question is how to best fix this. The abstraction of locations that def_use uses does not allow one to express "a subset of the bits of h.h.a".

[mihaibudiu]

Probably the problem is with the copy-out to the slice - it does not overwrite the whole field.

[jafingerhut]

Explode every bit<W> into W bit<1>'s ! :-)

I put the smiley on that because it is for most code probably a terrible idea to do this inside the compiler's implementation, for compilation performance/memory anyway. It does have the advantage of perhaps being a simple way to make def use analysis precise down to the level of each individual bit of state.

[fruffy]

I cannot reproduce this bug in the latest version, has it been fixed? I see that def_use is not used anymore.
Still reproducible.

[mihaibudiu]

My initial diagnosis about slices was wrong; this was a bug in the modelling of function calls.