Hi Tom, On Wed, May 13, 2020 at 11:42 PM Tom Rini <tr...@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.g...@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
