> From: Bruce Richardson [mailto:bruce.richard...@intel.com]
> Sent: Wednesday, 4 June 2025 11.49
>
> On Wed, Jun 04, 2025 at 11:43:01AM +0200, Morten Brørup wrote:
> > > From: Bruce Richardson [mailto:bruce.richard...@intel.com]
> > > Sent: Wednesday, 4 June 2025 11.32
> > >
> > > On Fri, May 30, 2025 at 02:57:15PM +0100, Anatoly Burakov wrote:
[...]
> > > > +#else
> > > > + /* for 32-byte descriptors only support SSE */
> > > > + switch (vec_level) {
> > > > + case CI_RX_VEC_LEVEL_AVX512:
> >
> > If you are respinning this patch, add "/* fall through */" here.
> >
> I disagree, it's not needed and will only make the code less readable.
>
> > > > + case CI_RX_VEC_LEVEL_AVX2:
> >
> > And here.
> >
> > > > + case CI_RX_VEC_LEVEL_SSE:
> > > > + _ci_rxq_rearm_sse(rxq);
> > > > + break;
> > > > + }
> > > > +#endif /* RTE_NET_INTEL_USE_16BYTE_DESC */
> > > <snip>
> >
> > Please try building with 32-byte descriptors; the compiler should
> have complained about the implicit fall through.
> >
> The fallthrough tags are not necessary where you have a list of tags
> without any statements in between. It's common practice to have a
> couple of
> values match a single switch arm, and the fallthrough is obvious in
> those
> cases, since there are no statements between them. Once you start
> having
> some code for a case, then you need it, since you need to make it clear
> that it's not accidental.
Thank you for clarifying, Bruce.
The GCC documentation [1] doesn't mention any exception for cases without
statements, but Godbolt confirms it (not only for C11, but also for C23), so I
agree with Bruce about omitting them for readability.
Sorry about the noise. ;-)
[1]: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
