p4lang / p4-spec

Apache License 2.0
178 stars 80 forks source link

Allow easier casting of serializable enum to a different bit width #1242

Closed qobilidop closed 1 year ago

qobilidop commented 1 year ago

Currently a serializable enum can only be cast to its base type. So if we want to cast it to a different bit width, it has to happen in 2 steps, like the following:

(bit<new_W>) (bit<base_W>) serializable_enum

This feels unnecessarily verbose.

In a similar spirit to #992, I wonder if it would be good to allow casting a serializable enum to a different bit width in a single step like (bit<new_W>) serializable_enum?

mihaibudiu commented 1 year ago

There is a reason for having the cast be verbose, which is that on some architectures sign-extension is actually very expensive. So forcing the people to write the second cast makes it clear that there is some computation involved.

qobilidop commented 1 year ago

Thanks for the explanation! That reason I understand. But I wonder if the (bit<base_W>) serializable_enum casting indicates any extra computation cost though? My understanding is that this cast is mostly for frontend type checking, but no real work for backend.

If this is true, then (bit<new_W>) (bit<base_W>) serializable_enum should have the same computation overhead as (bit<new_W>) bit_W_val. Then does this justify simplifying it to (bit<new_W>) serializable_enum? Because I think the computation cost is already indicated by the second cast alone, and by not allowing implicitly casting serializable enum to other types (except for its underlying type).

qobilidop commented 1 year ago

Actually, when I double check the language spec, I noticed this sentence (in the section on implicit casts):

P4 only implicitly casts from int to fixed-width types and from enums with an underlying type to the underlying type.

Then I wonder if this implicit cast rule could apply in this case? I could be wrong, but I think a compiler pass like the following might be reasonable:

qobilidop commented 1 year ago

And I see the implicit cast rules are being discussed in #1223. Maybe this is a relevant case to that discussion.

jnfoster commented 1 year ago

Given that this convesation is linked to #1223, I'm going to close this in favor of that issue.