InlineActions also seems to handle exit statements incorrectly

[fruffy]

I have another instance where there are issues with exit, this time in InlineActions.

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
    @name("do_action") action do_action_0(inout bit<8> val_0) {
        val_0 = 8w1;
        exit;
    }
    @name("do_action_ingress") action do_action_ingress_0() {
        do_action_0(h.h.a);
    }
    apply {
        do_action_ingress_0();
    }
}

is converted to

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
    @name("do_action_ingress") action do_action_ingress_0() {
        {
            bit<8> val = h.h.a;
            val = 8w1;
            exit;
            h.h.a = val;
        }
    }
    apply {
        do_action_ingress_0();
    }
}

Should I collect all the examples of exit problems in one thread or keep filing individual issues?
inline_actions_exit.p4.txt
inline_actions_exit.stf.txt

[mihaibudiu]

It is more useful to have these in one place; if the fix is common to all of them we should have all these examples as regression tests. I would at least cross-link from the original issue to this one.

[mihaibudiu]

Tagged as duplicate, but kept for the test case.

[hesingh]

What is expected output for this issue with a fixed p4c?

Should the "exit;" just disappear from the output?

[mihaibudiu]

For this particular program removing exit would preserve the semantics.

[hesingh]

@mbudiu-vmw has recommended the fix should be in the SideEffectsOrdering pass to remove exit statements. I made minor changes to the SideEffectsOrdering pass here and this issue is fixed. With the changes, #2358 is also fixed.

What is left to fix is nested exit in action and this issue

[hesingh]

Also, even if the bmv2 backend supports exit statement, it does make sense to me that exit statements inside actions are removed in a frontend pass such as SideEffectsOrdering.

[ChrisDodd]

Some targets may support exit directly (as bmv2 does), and, on such targets, adding and testing a flag may introduce significant overhead. So we don't want to force removal of exits in the compiler frontend, as supporting such targets efficiently would become much harder.

[mihaibudiu]

It's not clear that in the most general case we can take advantage of native exit implementations - exactly because we have no way to execute the copy-out code on exit. My plan is to use the native implementation only when copy-out is not needed, but eliminate the other instances of exit in software.

[hesingh]

Agree with @mbudiu-vmw. I did mention the SideEffectsOrdering pass which deals with copy-out and thus only for such cases exit should be removed.