[PATCH 1/1] tools: ftdgrep: use /* fallthrough */ as needed

[Masahiro Yamada]

Hi Tom,

On Wed, May 13, 2020 at 11:42 PM Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, May 12, 2020 at 11:04:38PM -0400, Tom Rini wrote:
> > On Mon, May 11, 2020 at 09:08:03PM +0200, Heinrich Schuchardt wrote:
> > > On 5/11/20 8:40 PM, Tom Rini wrote:
> > > > On Sun, May 10, 2020 at 10:12:07PM +0900, Masahiro Yamada wrote:
> > > >> On Sun, May 10, 2020 at 12:12 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > >>>
> > > >>> GCC recognizes /* fallthrough */ if -Wimplicit-fallthrough=3 is enabled.
> > > >>
> > > >> FYI.
> > > >>
> > > >> Linux decided to not use /* fallthrough */ any more
> > > >> because Clang does not recognize it.
> > > >>
> > > >> __attribute__((__fallthrough__)) is supported
> > > >> by both Clang and recent GCC.
> > > In fact Linux has a define:
> > >
> > > include/linux/compiler_attributes.h:200:# define fallthrough
> > >         __attribute__((__fallthrough__))
> > >
> > > And in the code you would use
> > >
> > >     case foo:
> > >             fallthrough;
> > >     case bar:
> > >
> > > But the Linux kernel still has a lot of lines with
> > >
> > > /* fallthrough */
> > >
> > > Documentation/process/deprecated.rst:
> > >
> > > <cite>
> > > As there have been a long list of flaws `due to missing "break"
> > > statements <https://cwe.mitre.org/data/definitions/484.html>`_, we no
> > > longer allow implicit fall-through. In order to identify intentional
> > > fall-through cases, we have adopted a pseudo-keyword macro "fallthrough"
> > > which expands to gcc's extension `__attribute__((__fallthrough__))
> > > <https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_. (When
> > > the C17/C18  `[[fallthrough]]` syntax is more commonly supported by C
> > > compilers, static analyzers, and IDEs, we can switch to using that
> > > syntax for the macro pseudo-keyword.)
> > > </cite>
> > >
> > > Using the attribute is not standard C and not any better than using the
> > > comment. The real target is the C17 syntax.
> > >
> > > >>
> > > >>
> > > >> Linux is now doing treewide conversion
> > > >> from /* fallthrough */ to 'fallthrough;'.
> > > >>
> > > >> See include/linux/compiler_attributes.h in Linux.
> > > >>
> > > >> I do not know if U-Boot wants to align with it.
> > > >> (up to Tom ?)
> > > >
> > > > A re-sync on the compiler headers again and making use of this sounds
> > > > like a good idea, yes.
> > > >
> > >
> > > We should enable -Wimplicit-fallthrough like the kernel does. This
> > > defaults to -Wimplicit-fallthrough=3 and is happy with both the comment
> > > as well as with the attribute.
> > >
> > > @Tom:
> > > Will you update the compiler headers within this release cycle?
> > > Otherwise we should take the patch as is to get us closer to the
> > > -Wimplicit-fallthrough target.
> >
> > I'm not going to update it for this release cycle.  I've done the
> > initial import and build and there's some fairly large changes related
> > to inlining that I want to look at harder to see if we can/should do
> > something about (I don't want to derail this thread, I'll start
> > another).  But it's very far from zero size change and given the inline
> > changes I think it'll need real testing.
> >
> > And since the kernel isn't making a huge use yet of fallthrough; we can
> > afford to look a little harder at things.
>
> I think I've figured out the inline issue which is that we need
> scripts/Kconfig.include from the kernel, CC_HAS_ASM_INLINE Kconfig
> option, and re-sync with Kconfiglib, but that's still going to be enough
> stuff that I don't think it's good to pull in at -rc2.
>


I do not get how 'asm inline' support is related
to this topic.

GCC 9 started to support 'asm inline' for the better inlining heuristic.
The kernel uses a bunch of inline assembly
that is not as expensive as it looks.

As GCC is agnostic about the real cost of inline assembly,
'asm inline' is a good hint if people know the real cost is quite small.
Then, GCC will be able to inline more functions.

I do not know how important it is for U-Boot, though.

What is causing you a trouble?




--
Best Regards
Masahiro Yamada