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

warning for unsigned fixed width shift left is twice #3289

Closed
apinski-cavium opened this issue May 6, 2022 · 1 comment · Fixed by #3715
Closed

warning for unsigned fixed width shift left is twice #3289

apinski-cavium opened this issue May 6, 2022 · 1 comment · Fixed by #3715
Labels
enhancement This topic discusses an improvement to existing compiler code. fixed This topic is considered to be fixed.

Comments

@apinski-cavium
Copy link

Take:

bit<5> f(in int<3> a)
{
  return (5w1) << 6;
}

p4c produces:

shiftoverflow.p4(3): [--Wwarn=overflow] warning: <<: Shifting 5-bit value with 6
  return (5w1) << 6;
         ^^^^^^^^^^
shiftoverflow.p4(3): [--Wwarn=mismatch] warning: 5w64: value does not fit in 5 bits
  return (5w1) << 6;
         ^^^^^^^^^^

If we change 6 to 5, the first warning is gone but maybe it should not be; I think there is an off by one error.

The second warning is another interesting one as I thought shifts have a well defined behavior always. How can there be a mismatch?
Also the specification only talks about overflow for the signed case and no overflow for the unsigned case so both of these warnings should not be there.

@mihaibudiu mihaibudiu added the enhancement This topic discusses an improvement to existing compiler code. label May 19, 2022
@mihaibudiu
Copy link
Contributor

Perhaps the spec can be improved, but I think from an usability point of view all these warnings are useful.

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

Successfully merging a pull request may close this issue.

2 participants