Fix wording of set operations in spec

[jnfoster]

There are some inconsistencies about which types are supported (bit<n>, int<n>, serializable enums etc.)

[MollyDream]

I have tested set operations in BMV2, and there are many discrepancies between the compiler behavior and the spec.

• The spec restrains the operands of &&& to only bit<W> and serializable enum, but in Section 13.2.1.4, there are many examples using int as the operands, such as 0x1111 &&& 0xF000. int &&& int is also allowed in the BMV2 compiler.
• This constraint also makes it impossible to define masked key expressions for int<W> type keys since the compiler cannot unify a key expression of the type bit<W> with a key of the type int<W>. BMV2 allows int<W> &&& int<W>.
• Similarly, the spec restrains the operands of .. to only bit<W> and int<W>, but the BMV2 compiler also accepts int .. int and enum .. enum.
• For the three issues above, I would suggest making int<W>, bit<W>, int and enum all valid operands. Given that these four types have appeared several times as the the set of valid operands (also as header stack index and bit-slicing index), I agree with Andy that we should place them in a subsection and reference the subsection when needed.
• One last thing is about the implicit casts for the mask and range operators. Since 8.13 Operations on sets is way behind the section of 8.9 Casts, it is vague if they allow implicit casts of the operands. I think the answer is yes, but it should be clarified. I wonder if section 8.13 can be moved before 8.9. There are two reasons I propose this change:
  • Even though 8.13 has the title "operations on sets", mask and range operators still operate on bit<W>, int<W>, int and enum, which are sections 8.5-8.7. Putting 8.13 before 8.9 will make 8.9 more closely related to 8.5-8.7 and make explaining the scope of implicit casts more natural.
  • 8.13 currently is in a very awkward position in between 8.12 Structure-valued expressions and 8.14 Operations on struct types, breaking the flow of using structure-valued expressions to initialize struct types (a flow similar to 8.10 Operations on tuples expressions <- 8.11 Operations on lists). However, there is no reason that 8.13 needs to be there. Actually, 8.13 is a quite isolated section that can be moved easily. If moving it to before 8.9 does not sound right, I would suggest moving it to after 8.18 Operations on header unions.

BMV2 code snippet:

header hdr {
    bit<16> input;
    int<16> output;
}

enum bit<16> E2 {
  e3 = 1,
  e4 = 2
}

const bit<16> k1 = 1;
const int<16> k2 = 1;
const int k3 = 1;
const E2 k4 = E2.e3;


control compute(inout hdr h) {
    table table_test {
        key = { h.input : ternary; 
                h.output : ternary; }
        actions = { NoAction; }
        const entries = {
            (k1 &&& k1, _)    : NoAction();
            (k4 &&& k4, _)    : NoAction();
            (k1 &&& k4, _)    : NoAction();
            (k4 &&& k1, _)    : NoAction();
            (k3 &&& k3, _)    : NoAction(); // Spec does not allow int in masking operation.
            (k3 &&& k1, _)    : NoAction(); // Spec does not allow implicit casts in masking operation.
            (_ , k2 &&& k2)   : NoAction(); // Spec does not allow int<W> in masking operation.
            (_ , k2 &&& k3)   : NoAction(); // Spec does not allow int<W> in masking operation.
        }
    }

    table table_test_2 {
        key = { h.input : range; 
                h.output : range; }
        actions = { NoAction; }
        const entries = {
            (k1 .. k1, _)    : NoAction();
            (_ , k2 .. k2)   : NoAction();
            (k4 .. k4, _)    : NoAction(); // Spec does not allow enum in range operation.
            (k1 .. k4, _)    : NoAction(); // Spec does not allow enum in range operation.
            (k4 .. k1, _)    : NoAction(); // Spec does not allow enum in range operation.
            (k3 .. k3, _)    : NoAction(); // Spec does not allow int in range operation.
            (k3 .. k1, _)    : NoAction(); // Spec does not allow implicit casts in masking operation.
            (_ , k2 .. k3)   : NoAction(); // Spec does not allow implicit casts in masking operation.
        }
    }

    apply {
        table_test.apply();
        table_test_2.apply();
    }
}

[vgurevich]

@MollyDream

The spec is correct -- the type of the arguments of &&& should be of the same specific width . As a general rule, however, the language allows implicit casts from int into bit<N> and that's why the code you are referring to compiles cleanly.

Same is true about using serializable enums -- the language supports implicit casts into the base type.

I do not believe the spec change is warranted here

[MollyDream]

Hi @vgurevich Thank you for your reply. I don't think that implicit casts should be applied recursively into the operations based on what the spec said in 8.9.2. For example, for an expression of 8w1 + (3 + 4), after adding the implicit cast, it should be 8w1 + (bit<8>)(3 + 4) instead of 8w1 + ((bit<8>)3 + (bit<8>)4). Similarly here, if we have a key mask expression of 1 &&& 1 for a bit<8> key, the implicit casts should produce an expression of (bit<8>) (1 &&& 1) instead of ((bit<8>) 1) &&& ((bit<8>) 1). That is why I think the spec is a bit strange in saying that "the infix operator &&& takes two arguments of type bit<W> or serializable enum", excluding int and int<w>. Similarly for the .. operator. The spec also seems a bit strange in listing different allowed types for &&& and .. given that these two operators accepts the same set of types as operands. Lastly, I think this section 8.13 is after the 8.9.2 implicit casts, so it is very vague if the binary operations that allow implicit casts include these two operations.

[mihaibudiu]

Is this fixed by #1008?

[jnfoster]

In the interest of tidying up the set of active issues on the P4 specification repository, I'm marking this as "stalled" and closing it. Of course, we can always re-open it in the future if there is interest in resurrecting it.