[PATCH -next] fbdev: c2p: Fix link failure on non-inlining

[PATCH -next] fbdev: c2p: Fix link failure on non-inlining

[Geert Uytterhoeven]

When the compiler decides not to inline the Chunky-to-Planar core
functions, the build fails with:

    c2p_planar.c:(.text+0xd6): undefined reference to `c2p_unsupported'
    c2p_planar.c:(.text+0x1dc): undefined reference to `c2p_unsupported'
    c2p_iplan2.c:(.text+0xc4): undefined reference to `c2p_unsupported'
    c2p_iplan2.c:(.text+0x150): undefined reference to `c2p_unsupported'

Fix this by marking the functions __always_inline.

Reported-by: noreply@ellerman.id.au
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Fixes: 025f072e5823947c ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")

As this is a patch in akpm's tree, the commit ID in the Fixes tag is not
stable.
---
 drivers/video/fbdev/c2p_core.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/c2p_core.h b/drivers/video/fbdev/c2p_core.h
index e1035a865fb945f0..45a6d895a7d7208e 100644
--- a/drivers/video/fbdev/c2p_core.h
+++ b/drivers/video/fbdev/c2p_core.h
@@ -29,7 +29,7 @@ static inline void _transp(u32 d[], unsigned int i1, unsigned int i2,
 
 extern void c2p_unsupported(void);
 
-static inline u32 get_mask(unsigned int n)
+static __always_inline u32 get_mask(unsigned int n)
 {
 	switch (n) {
 	case 1:
@@ -57,7 +57,7 @@ static inline u32 get_mask(unsigned int n)
      *  Transpose operations on 8 32-bit words
      */
 
-static inline void transp8(u32 d[], unsigned int n, unsigned int m)
+static __always_inline void transp8(u32 d[], unsigned int n, unsigned int m)
 {
 	u32 mask = get_mask(n);
 
@@ -99,7 +99,7 @@ static inline void transp8(u32 d[], unsigned int n, unsigned int m)
      *  Transpose operations on 4 32-bit words
      */
 
-static inline void transp4(u32 d[], unsigned int n, unsigned int m)
+static __always_inline void transp4(u32 d[], unsigned int n, unsigned int m)
 {
 	u32 mask = get_mask(n);
 
@@ -126,7 +126,7 @@ static inline void transp4(u32 d[], unsigned int n, unsigned int m)
      *  Transpose operations on 4 32-bit words (reverse order)
      */
 
-static inline void transp4x(u32 d[], unsigned int n, unsigned int m)
+static __always_inline void transp4x(u32 d[], unsigned int n, unsigned int m)
 {
 	u32 mask = get_mask(n);
 
-- 
2.17.1

Re: [PATCH -next] fbdev: c2p: Fix link failure on non-inlining

[Masahiro Yamada]

On Thu, Sep 26, 2019 at 7:13 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> When the compiler decides not to inline the Chunky-to-Planar core
> functions, the build fails with:
>
>     c2p_planar.c:(.text+0xd6): undefined reference to `c2p_unsupported'
>     c2p_planar.c:(.text+0x1dc): undefined reference to `c2p_unsupported'
>     c2p_iplan2.c:(.text+0xc4): undefined reference to `c2p_unsupported'
>     c2p_iplan2.c:(.text+0x150): undefined reference to `c2p_unsupported'
>
> Fix this by marking the functions __always_inline.
>
> Reported-by: noreply@ellerman.id.au
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>


Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>




-- 
Best Regards
Masahiro Yamada

Re: [PATCH -next] fbdev: c2p: Fix link failure on non-inlining

[Masahiro Yamada]

On Thu, Sep 26, 2019 at 7:13 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> When the compiler decides not to inline the Chunky-to-Planar core
> functions, the build fails with:
>
>     c2p_planar.c:(.text+0xd6): undefined reference to `c2p_unsupported'
>     c2p_planar.c:(.text+0x1dc): undefined reference to `c2p_unsupported'
>     c2p_iplan2.c:(.text+0xc4): undefined reference to `c2p_unsupported'
>     c2p_iplan2.c:(.text+0x150): undefined reference to `c2p_unsupported'
>
> Fix this by marking the functions __always_inline.
>
> Reported-by: noreply@ellerman.id.au
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Fixes: 025f072e5823947c ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
>
> As this is a patch in akpm's tree, the commit ID in the Fixes tag is not
> stable.

BTW, that Fixes tag is incorrect.

Irrespective of 025f072e5823947c, you could manually enable
CONFIG_OPTIMIZE_INLINING from menuconfig etc.

So, this build error would have been found much earlier
if somebody had been running randconfig tests on m68k.

It is impossible to detect this error on other architectures
because the driver config options are guarded by
'depends on ATARI' or 'depends on AMIGA'.


The correct tag is:

Fixes: 9012d011660e ("compiler: allow all arches to enable
CONFIG_OPTIMIZE_INLINING")

The commit id is stable.



As an additional work,
depends on (AMIGA || COMPILE_TEST)
would be nice unless this driver contains m68k-specific code.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH -next] fbdev: c2p: Fix link failure on non-inlining

[Geert Uytterhoeven]

Hi Yamada-san,

On Thu, Sep 26, 2019 at 12:45 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> On Thu, Sep 26, 2019 at 7:13 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > When the compiler decides not to inline the Chunky-to-Planar core
> > functions, the build fails with:
> >
> >     c2p_planar.c:(.text+0xd6): undefined reference to `c2p_unsupported'
> >     c2p_planar.c:(.text+0x1dc): undefined reference to `c2p_unsupported'
> >     c2p_iplan2.c:(.text+0xc4): undefined reference to `c2p_unsupported'
> >     c2p_iplan2.c:(.text+0x150): undefined reference to `c2p_unsupported'
> >
> > Fix this by marking the functions __always_inline.
> >
> > Reported-by: noreply@ellerman.id.au
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > ---
> > Fixes: 025f072e5823947c ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
> >
> > As this is a patch in akpm's tree, the commit ID in the Fixes tag is not
> > stable.
>
> BTW, that Fixes tag is incorrect.
>
> Irrespective of 025f072e5823947c, you could manually enable
> CONFIG_OPTIMIZE_INLINING from menuconfig etc.

Merely enabling that doesn't help.
You also need CONFIG_CC_OPTIMIZE_FOR_SIZE=y, while the
default is CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y.
Which is why my all{mod,yes}config builds never caught that :-(

> So, this build error would have been found much earlier
> if somebody had been running randconfig tests on m68k.

It's been a while I did that...

BTW, does randconfig randomize choices these days?
I remember it didn't use to do that.

> It is impossible to detect this error on other architectures
> because the driver config options are guarded by
> 'depends on ATARI' or 'depends on AMIGA'.
>
> The correct tag is:
>
> Fixes: 9012d011660e ("compiler: allow all arches to enable
> CONFIG_OPTIMIZE_INLINING")
>
> The commit id is stable.

Thanks, will update.

> As an additional work,
> depends on (AMIGA || COMPILE_TEST)
> would be nice unless this driver contains m68k-specific code.

The Amiga and Atari frame buffer drivers need <asm/{amiga,atari}hw.h>,
and the Atari driver contains inline asm.

The C2P code could be put behind its own Kconfig symbol, I guess.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH -next] fbdev: c2p: Fix link failure on non-inlining

[Masahiro Yamada]

Hi Geert,

On Thu, Sep 26, 2019 at 8:43 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

>
> BTW, does randconfig randomize choices these days?
> I remember it didn't use to do that.

randconfig does randomize choices.


masahiro@pug:~/ref/linux$ make -s randconfig ; grep OPTIMIZE_FOR .config
KCONFIG_SEED=0x75F1F6C8
CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y
# CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
masahiro@pug:~/ref/linux$ make -s randconfig ; grep OPTIMIZE_FOR .config
KCONFIG_SEED=0x8FDFC7FC
# CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set
CONFIG_CC_OPTIMIZE_FOR_SIZE=y


all{yes,mod}config always takes the default in the choice.
So, you cannot enable CONFIG_CC_OPTIMIZE_FOR_SIZE by all{yes,mod}config.





> The Amiga and Atari frame buffer drivers need <asm/{amiga,atari}hw.h>,
> and the Atari driver contains inline asm.
>
> The C2P code could be put behind its own Kconfig symbol, I guess.

OK, then.


Thanks.




-- 
Best Regards
Masahiro Yamada