undefined reference to `__multi3' when building with gcc 7.x

undefined reference to `__multi3' when building with gcc 7.x

[Thomas Petazzoni]

[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]

Hello,

When trying to build the current Linux master with a gcc 7.x toolchain
for mips64r6-n32, I'm getting the following build failure:

crypto/scompress.o: In function `.L31':
scompress.c:(.text+0x2a0): undefined reference to `__multi3'
drivers/base/component.o: In function `.L97':
component.c:(.text+0x4a4): undefined reference to `__multi3'
drivers/base/component.o: In function `component_master_add_with_match':
component.c:(.text+0x8c4): undefined reference to `__multi3'
net/core/ethtool.o: In function `ethtool_set_per_queue_coalesce':
ethtool.c:(.text+0x1ab0): undefined reference to `__multi3'
Makefile:1000: recipe for target 'vmlinux' failed
make[2]: *** [vmlinux] Error 1

Taking the example from net/core/ethtool.o, objdump says:

    1aac:       00408025        move    s0,v0
    1ab0:       e8000000        balc    1ab4 <ethtool_set_per_queue_coalesce+0x7c>
    1ab4:       14600000        bnez    v1,1ab8 <ethtool_set_per_queue_coalesce+0x80>

And readelf tells us:

Relocation section '.rela.text' at offset 0xaa00 contains 1189 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
[...]
000000001ab0  023a0000003d R_MIPS_PC26_S2    0000000000000000 __multi3 - 4
                    Type2: R_MIPS_NONE      
                    Type3: R_MIPS_NONE      
[...]
Symbol table '.symtab' contains 586 entries:
[...]
   570: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  UND __multi3

__multi3() is normally provided by libgcc, but of course the kernel
doesn't link with libgcc.

The bug can be reproduced by building with the toolchain available at
http://toolchains.free-electrons.com/downloads/2017.08-rc1-fix-binutils/toolchains/mips64r6el-n32/tarballs/mips64r6el-n32--glibc--bleeding-edge-2017.05-1453-ga703fdd-1.tar.bz2
and building with the attached kernel configuration file.

It is not clear to me if this is a kernel issue (lack of __multi3 in
arch/mips/lib/), or a gcc bug in that it shouldn't emit a call to this
function.

FWIW, sparc64 had a similar issue, and they added __multi3 in their
libgcc replacement, see commit
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sparc/lib?id=1b4af13ff2cc6897557bb0b8d9e2fad4fa4d67aa.

Best regards,

Thomas Petazzoni
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: defconfig --]
[-- Type: application/octet-stream, Size: 654 bytes --]

CONFIG_MIPS_MALTA=y
CONFIG_CPU_LITTLE_ENDIAN=y
CONFIG_CPU_MIPS64_R6=y
CONFIG_64BIT=y
CONFIG_SYSVIPC=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
CONFIG_PCI=y
CONFIG_MIPS32_O32=y
CONFIG_MIPS32_N32=y
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
CONFIG_INET=y
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
CONFIG_IDE=y
CONFIG_BLK_DEV_IDECD=y
CONFIG_IDE_GENERIC=y
CONFIG_BLK_DEV_GENERIC=y
CONFIG_BLK_DEV_PIIX=y
CONFIG_NETDEVICES=y
CONFIG_PCNET32=y
CONFIG_INPUT_EVDEV=y
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_FB=y
CONFIG_FB_CIRRUS=y
CONFIG_USB=y
CONFIG_USB_UHCI_HCD=y
CONFIG_EXT4_FS=y
CONFIG_TMPFS=y
CONFIG_TMPFS_POSIX_ACL=y

Re: undefined reference to `__multi3' when building with gcc 7.x

[Ralf Baechle]

On Thu, Aug 03, 2017 at 10:55:47PM +0200, Thomas Petazzoni wrote:

> Hello,
> 
> When trying to build the current Linux master with a gcc 7.x toolchain
> for mips64r6-n32, I'm getting the following build failure:
> 
> crypto/scompress.o: In function `.L31':
> scompress.c:(.text+0x2a0): undefined reference to `__multi3'
> drivers/base/component.o: In function `.L97':
> component.c:(.text+0x4a4): undefined reference to `__multi3'
> drivers/base/component.o: In function `component_master_add_with_match':
> component.c:(.text+0x8c4): undefined reference to `__multi3'
> net/core/ethtool.o: In function `ethtool_set_per_queue_coalesce':
> ethtool.c:(.text+0x1ab0): undefined reference to `__multi3'
> Makefile:1000: recipe for target 'vmlinux' failed
> make[2]: *** [vmlinux] Error 1
> 
> Taking the example from net/core/ethtool.o, objdump says:
> 
>     1aac:       00408025        move    s0,v0
>     1ab0:       e8000000        balc    1ab4 <ethtool_set_per_queue_coalesce+0x7c>
>     1ab4:       14600000        bnez    v1,1ab8 <ethtool_set_per_queue_coalesce+0x80>
> 
> And readelf tells us:
> 
> Relocation section '.rela.text' at offset 0xaa00 contains 1189 entries:
>   Offset          Info           Type           Sym. Value    Sym. Name + Addend
> [...]
> 000000001ab0  023a0000003d R_MIPS_PC26_S2    0000000000000000 __multi3 - 4
>                     Type2: R_MIPS_NONE      
>                     Type3: R_MIPS_NONE      
> [...]
> Symbol table '.symtab' contains 586 entries:
> [...]
>    570: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  UND __multi3
> 
> __multi3() is normally provided by libgcc, but of course the kernel
> doesn't link with libgcc.
> 
> The bug can be reproduced by building with the toolchain available at
> http://toolchains.free-electrons.com/downloads/2017.08-rc1-fix-binutils/toolchains/mips64r6el-n32/tarballs/mips64r6el-n32--glibc--bleeding-edge-2017.05-1453-ga703fdd-1.tar.bz2
> and building with the attached kernel configuration file.
> 
> It is not clear to me if this is a kernel issue (lack of __multi3 in
> arch/mips/lib/), or a gcc bug in that it shouldn't emit a call to this
> function.
> 
> FWIW, sparc64 had a similar issue, and they added __multi3 in their
> libgcc replacement, see commit
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sparc/lib?id=1b4af13ff2cc6897557bb0b8d9e2fad4fa4d67aa.

I think these days we've given up the stuborn resistance of the old days
against adding new libgcc1 functions to the kernel, so we should probably
just add it.

I'm looking into this but a small wrench into the gear is that I'm still
on GCC 6 so I'm off to building myself a cross-gcc first ...

  Ralf

Re: undefined reference to `__multi3' when building with gcc 7.x

[Ralf Baechle]

On Fri, Aug 04, 2017 at 02:05:57AM +0200, Ralf Baechle wrote:

> > 
> > When trying to build the current Linux master with a gcc 7.x toolchain
> > for mips64r6-n32, I'm getting the following build failure:
> > 
> > crypto/scompress.o: In function `.L31':
> > scompress.c:(.text+0x2a0): undefined reference to `__multi3'
> > drivers/base/component.o: In function `.L97':
> > component.c:(.text+0x4a4): undefined reference to `__multi3'
> > drivers/base/component.o: In function `component_master_add_with_match':
> > component.c:(.text+0x8c4): undefined reference to `__multi3'
> > net/core/ethtool.o: In function `ethtool_set_per_queue_coalesce':
> > ethtool.c:(.text+0x1ab0): undefined reference to `__multi3'
> > Makefile:1000: recipe for target 'vmlinux' failed
> > make[2]: *** [vmlinux] Error 1
> > 
> > Taking the example from net/core/ethtool.o, objdump says:
> > 
> >     1aac:       00408025        move    s0,v0
> >     1ab0:       e8000000        balc    1ab4 <ethtool_set_per_queue_coalesce+0x7c>
> >     1ab4:       14600000        bnez    v1,1ab8 <ethtool_set_per_queue_coalesce+0x80>
> > 
> > And readelf tells us:
> > 
> > Relocation section '.rela.text' at offset 0xaa00 contains 1189 entries:
> >   Offset          Info           Type           Sym. Value    Sym. Name + Addend
> > [...]
> > 000000001ab0  023a0000003d R_MIPS_PC26_S2    0000000000000000 __multi3 - 4
> >                     Type2: R_MIPS_NONE      
> >                     Type3: R_MIPS_NONE      
> > [...]
> > Symbol table '.symtab' contains 586 entries:
> > [...]
> >    570: 0000000000000000     0 OBJECT  GLOBAL DEFAULT  UND __multi3
> > 
> > __multi3() is normally provided by libgcc, but of course the kernel
> > doesn't link with libgcc.
> > 
> > The bug can be reproduced by building with the toolchain available at
> > http://toolchains.free-electrons.com/downloads/2017.08-rc1-fix-binutils/toolchains/mips64r6el-n32/tarballs/mips64r6el-n32--glibc--bleeding-edge-2017.05-1453-ga703fdd-1.tar.bz2
> > and building with the attached kernel configuration file.
> > 
> > It is not clear to me if this is a kernel issue (lack of __multi3 in
> > arch/mips/lib/), or a gcc bug in that it shouldn't emit a call to this
> > function.
> > 
> > FWIW, sparc64 had a similar issue, and they added __multi3 in their
> > libgcc replacement, see commit
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sparc/lib?id=1b4af13ff2cc6897557bb0b8d9e2fad4fa4d67aa.
> 
> I think these days we've given up the stuborn resistance of the old days
> against adding new libgcc1 functions to the kernel, so we should probably
> just add it.
> 
> I'm looking into this but a small wrench into the gear is that I'm still
> on GCC 6 so I'm off to building myself a cross-gcc first ...

I now can reproduce the issue with vanilla FSF binutils 2.28 and GCC 7.1.0.

  Ralf

Re: undefined reference to `__multi3' when building with gcc 7.x

[Thomas Petazzoni]

Hello,

On Fri, 4 Aug 2017 17:19:20 +0200, Ralf Baechle wrote:

> > > The bug can be reproduced by building with the toolchain available at
> > > http://toolchains.free-electrons.com/downloads/2017.08-rc1-fix-binutils/toolchains/mips64r6el-n32/tarballs/mips64r6el-n32--glibc--bleeding-edge-2017.05-1453-ga703fdd-1.tar.bz2
> > > and building with the attached kernel configuration file.
> > > 
> > > It is not clear to me if this is a kernel issue (lack of __multi3 in
> > > arch/mips/lib/), or a gcc bug in that it shouldn't emit a call to this
> > > function.
> > > 
> > > FWIW, sparc64 had a similar issue, and they added __multi3 in their
> > > libgcc replacement, see commit
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sparc/lib?id=1b4af13ff2cc6897557bb0b8d9e2fad4fa4d67aa.  
> > 
> > I think these days we've given up the stuborn resistance of the old days
> > against adding new libgcc1 functions to the kernel, so we should probably
> > just add it.
> > 
> > I'm looking into this but a small wrench into the gear is that I'm still
> > on GCC 6 so I'm off to building myself a cross-gcc first ...  
> 
> I now can reproduce the issue with vanilla FSF binutils 2.28 and GCC 7.1.0.

Great! However, looking at the functions that end up calling __multi3,
I'm wondering why suddenly gcc 7.x needs to call such a function, while
the same code was compiling without __multi3 in libgcc with gcc 6.x.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: undefined reference to `__multi3' when building with gcc 7.x

[Ralf Baechle]

On Fri, Aug 04, 2017 at 05:41:51PM +0200, Thomas Petazzoni wrote:

> > > > The bug can be reproduced by building with the toolchain available at
> > > > http://toolchains.free-electrons.com/downloads/2017.08-rc1-fix-binutils/toolchains/mips64r6el-n32/tarballs/mips64r6el-n32--glibc--bleeding-edge-2017.05-1453-ga703fdd-1.tar.bz2
> > > > and building with the attached kernel configuration file.
> > > > 
> > > > It is not clear to me if this is a kernel issue (lack of __multi3 in
> > > > arch/mips/lib/), or a gcc bug in that it shouldn't emit a call to this
> > > > function.
> > > > 
> > > > FWIW, sparc64 had a similar issue, and they added __multi3 in their
> > > > libgcc replacement, see commit
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sparc/lib?id=1b4af13ff2cc6897557bb0b8d9e2fad4fa4d67aa.  
> > > 
> > > I think these days we've given up the stuborn resistance of the old days
> > > against adding new libgcc1 functions to the kernel, so we should probably
> > > just add it.
> > > 
> > > I'm looking into this but a small wrench into the gear is that I'm still
> > > on GCC 6 so I'm off to building myself a cross-gcc first ...  
> > 
> > I now can reproduce the issue with vanilla FSF binutils 2.28 and GCC 7.1.0.
> 
> Great! However, looking at the functions that end up calling __multi3,
> I'm wondering why suddenly gcc 7.x needs to call such a function, while
> the same code was compiling without __multi3 in libgcc with gcc 6.x.

Chances are it's something specific to MIPS64 R6.  Before trying your
config file I also tried a number of other defconfigs and all built
well.

Here's a test case which generates a reference to __multi3:

unsigned long func(unsigned long a, unsigned long b)
{
        return a > (~0UL) / b;
}

GCC rearanges above statement to:

	return (unsigned __int128)a * (unsigned __int128) b > 0xffffffff;

computing which requires 128 bit intermediate results.  This is the
code generated for MIPS64 R2:

	dmultu	$4,$5
	mfhi	$2
	sltu	$2,$0,$2

And this is for R6:

	move	$2,$4
	move	$7,$5			# $6/$7 contain the second op
	move	$6,$0
	move	$4,$0
	move	$5,$2			# $4/$5 contain the first op
	sd	$31,8($sp)
	balc	__multi3
	sltu	$2,$0,$2

  Ralf

Re: undefined reference to `__multi3' when building with gcc 7.x

[Thomas Petazzoni]

Hello,

On Sat, 5 Aug 2017 00:25:00 +0200, Ralf Baechle wrote:

> > Great! However, looking at the functions that end up calling __multi3,
> > I'm wondering why suddenly gcc 7.x needs to call such a function, while
> > the same code was compiling without __multi3 in libgcc with gcc 6.x.  
> 
> Chances are it's something specific to MIPS64 R6.  Before trying your
> config file I also tried a number of other defconfigs and all built
> well.
> 
> Here's a test case which generates a reference to __multi3:
> 
> unsigned long func(unsigned long a, unsigned long b)
> {
>         return a > (~0UL) / b;
> }
> 
> GCC rearanges above statement to:
> 
> 	return (unsigned __int128)a * (unsigned __int128) b > 0xffffffff;

And this is normal/expected ?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: undefined reference to `__multi3' when building with gcc 7.x

[Ralf Baechle]

On Sat, Aug 05, 2017 at 01:56:49PM +0200, Thomas Petazzoni wrote:

> On Sat, 5 Aug 2017 00:25:00 +0200, Ralf Baechle wrote:
> 
> > > Great! However, looking at the functions that end up calling __multi3,
> > > I'm wondering why suddenly gcc 7.x needs to call such a function, while
> > > the same code was compiling without __multi3 in libgcc with gcc 6.x.  
> > 
> > Chances are it's something specific to MIPS64 R6.  Before trying your
> > config file I also tried a number of other defconfigs and all built
> > well.
> > 
> > Here's a test case which generates a reference to __multi3:
> > 
> > unsigned long func(unsigned long a, unsigned long b)
> > {
> >         return a > (~0UL) / b;
> > }
> > 
> > GCC rearanges above statement to:
> > 
> > 	return (unsigned __int128)a * (unsigned __int128) b > 0xffffffff;
> 
> And this is normal/expected ?

Without consideration of performance, It's certainly is valid code.  And
with that I can't drop the issue as a GCC code generation bug.

However it seems GCC itself doesn't seem to have a __multi3 in its
libgcc2 - which indeed would be a GCC issue - at least none I was easily
able to find with grep so I'm adding Matthew Fortune to cc in the hope he
can shed some light on this.

  Ralf

Re: undefined reference to `__multi3' when building with gcc 7.x

[Thomas Petazzoni]

Hello,

On Mon, 7 Aug 2017 10:34:48 +0200, Ralf Baechle wrote:

> > > Chances are it's something specific to MIPS64 R6.  Before trying your
> > > config file I also tried a number of other defconfigs and all built
> > > well.
> > > 
> > > Here's a test case which generates a reference to __multi3:
> > > 
> > > unsigned long func(unsigned long a, unsigned long b)
> > > {
> > >         return a > (~0UL) / b;
> > > }
> > > 
> > > GCC rearanges above statement to:
> > > 
> > > 	return (unsigned __int128)a * (unsigned __int128) b > 0xffffffff;  
> > 
> > And this is normal/expected ?  
> 
> Without consideration of performance, It's certainly is valid code.  And
> with that I can't drop the issue as a GCC code generation bug.
> 
> However it seems GCC itself doesn't seem to have a __multi3 in its
> libgcc2 - which indeed would be a GCC issue - at least none I was easily
> able to find with grep so I'm adding Matthew Fortune to cc in the hope he
> can shed some light on this.

Indeed, I don't see __multi3 implemented in libgcc in the source code,
but it's probably because it's tricky to see its implementation,
as it really is there:

$ ./bin/mips64el-linux-readelf -a ./mips64el-buildroot-linux-uclibc/sysroot/lib/libgcc_s.so.1 | grep multi3
  1747: 00011700   100 FUNC    GLOBAL DEFAULT   11 __multi3@@GCC_3.0
  5511: 00011700   100 FUNC    GLOBAL DEFAULT   11 __multi3
  000435e4 -32236(gp) 00011700 00011700 FUNC     11 __multi3

Objdump says:

00011700 <__multi3>:
   11700:       0006103e        dsrl32  v0,a2,0x0
   11704:       7c89f803        dext    a5,a0,0x0,0x20
   11708:       0004403e        dsrl32  a4,a0,0x0
   1170c:       7ccaf803        dext    a6,a2,0x0,0x20
   11710:       012a589c        dmul    a7,a5,a6
   11714:       010a509c        dmul    a6,a4,a6
   11718:       0122489c        dmul    a5,a5,v0
   1171c:       0102409c        dmul    a4,a4,v0
   11720:       012a482d        daddu   a5,a5,a6
   11724:       000b103e        dsrl32  v0,a7,0x0
   11728:       0049102d        daddu   v0,v0,a5
   1172c:       184a0003        bgeuc   v0,a6,1173c <.L2>
   11730:       24090001        li      a5,1
   11734:       0009483c        dsll32  a5,a5,0x0
   11738:       0109402d        daddu   a4,a4,a5

The same __multi3 problem also occurred on SPARC64, and they fixed it
by adding a __multi3 implementation in the kernel. See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sparc?id=1b4af13ff2cc6897557bb0b8d9e2fad4fa4d67aa.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: undefined reference to `__multi3' when building with gcc 7.x

[Ralf Baechle]

On Sun, Aug 13, 2017 at 10:46:02PM +0200, Thomas Petazzoni wrote:
> Date:   Sun, 13 Aug 2017 22:46:02 +0200
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> To: Ralf Baechle <ralf@linux-mips.org>
> Cc: Matthew Fortune <Matthew.Fortune@imgtec.com>,
>  linux-mips@linux-mips.org, Waldemar Brodkorb <wbx@openadk.org>
> Subject: Re: undefined reference to `__multi3' when building with gcc 7.x
> Content-Type: text/plain; charset=US-ASCII
> 
> Hello,
> 
> On Mon, 7 Aug 2017 10:34:48 +0200, Ralf Baechle wrote:
> 
> > > > Chances are it's something specific to MIPS64 R6.  Before trying your
> > > > config file I also tried a number of other defconfigs and all built
> > > > well.
> > > > 
> > > > Here's a test case which generates a reference to __multi3:
> > > > 
> > > > unsigned long func(unsigned long a, unsigned long b)
> > > > {
> > > >         return a > (~0UL) / b;
> > > > }
> > > > 
> > > > GCC rearanges above statement to:
> > > > 
> > > > 	return (unsigned __int128)a * (unsigned __int128) b > 0xffffffff;  
> > > 
> > > And this is normal/expected ?  
> > 
> > Without consideration of performance, It's certainly is valid code.  And
> > with that I can't drop the issue as a GCC code generation bug.
> > 
> > However it seems GCC itself doesn't seem to have a __multi3 in its
> > libgcc2 - which indeed would be a GCC issue - at least none I was easily
> > able to find with grep so I'm adding Matthew Fortune to cc in the hope he
> > can shed some light on this.
> 
> Indeed, I don't see __multi3 implemented in libgcc in the source code,
> but it's probably because it's tricky to see its implementation,
> as it really is there:
> 
> $ ./bin/mips64el-linux-readelf -a ./mips64el-buildroot-linux-uclibc/sysroot/lib/libgcc_s.so.1 | grep multi3
>   1747: 00011700   100 FUNC    GLOBAL DEFAULT   11 __multi3@@GCC_3.0
>   5511: 00011700   100 FUNC    GLOBAL DEFAULT   11 __multi3
>   000435e4 -32236(gp) 00011700 00011700 FUNC     11 __multi3
> 
> Objdump says:
> 
> 00011700 <__multi3>:
>    11700:       0006103e        dsrl32  v0,a2,0x0
>    11704:       7c89f803        dext    a5,a0,0x0,0x20
>    11708:       0004403e        dsrl32  a4,a0,0x0
>    1170c:       7ccaf803        dext    a6,a2,0x0,0x20
>    11710:       012a589c        dmul    a7,a5,a6
>    11714:       010a509c        dmul    a6,a4,a6
>    11718:       0122489c        dmul    a5,a5,v0
>    1171c:       0102409c        dmul    a4,a4,v0
>    11720:       012a482d        daddu   a5,a5,a6
>    11724:       000b103e        dsrl32  v0,a7,0x0
>    11728:       0049102d        daddu   v0,v0,a5
>    1172c:       184a0003        bgeuc   v0,a6,1173c <.L2>
>    11730:       24090001        li      a5,1
>    11734:       0009483c        dsll32  a5,a5,0x0
>    11738:       0109402d        daddu   a4,a4,a5

I happened to have a GCC build dir around so I greped for __multi3 and
found it hiding in muldi3.o.

Maybe that'obvious for those in the know, not me :)  Also that .o file
contained MIPS III code and I was able to get GCC to emit a reference
to __multi3 for MIPS III or MIPS64R1 targets, so version of __multi3
even seems unused.

  Ralf

RE: undefined reference to `__multi3' when building with gcc 7.x

[Matthew Fortune]

Ralf Baechle <ralf@linux-mips.org> writes:
> On Sun, Aug 13, 2017 at 10:46:02PM +0200, Thomas Petazzoni wrote:
> > Date:   Sun, 13 Aug 2017 22:46:02 +0200
> > From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > To: Ralf Baechle <ralf@linux-mips.org>
> > Cc: Matthew Fortune <Matthew.Fortune@imgtec.com>,
> >  linux-mips@linux-mips.org, Waldemar Brodkorb <wbx@openadk.org>
> > Subject: Re: undefined reference to `__multi3' when building with gcc 7.x
> > Content-Type: text/plain; charset=US-ASCII
> >
> > Hello,
> >
> > On Mon, 7 Aug 2017 10:34:48 +0200, Ralf Baechle wrote:
> >
> > > > > Chances are it's something specific to MIPS64 R6.  Before trying your
> > > > > config file I also tried a number of other defconfigs and all built
> > > > > well.
> > > > >
> > > > > Here's a test case which generates a reference to __multi3:
> > > > >
> > > > > unsigned long func(unsigned long a, unsigned long b)
> > > > > {
> > > > >         return a > (~0UL) / b;
> > > > > }
> > > > >
> > > > > GCC rearanges above statement to:
> > > > >
> > > > > 	return (unsigned __int128)a * (unsigned __int128) b > 0xffffffff;
> > > >
> > > > And this is normal/expected ?
> > >
> > > Without consideration of performance, It's certainly is valid code.  And
> > > with that I can't drop the issue as a GCC code generation bug.
> > >
> > > However it seems GCC itself doesn't seem to have a __multi3 in its
> > > libgcc2 - which indeed would be a GCC issue - at least none I was easily
> > > able to find with grep so I'm adding Matthew Fortune to cc in the hope he
> > > can shed some light on this.
> >
> > Indeed, I don't see __multi3 implemented in libgcc in the source code,
> > but it's probably because it's tricky to see its implementation,
> > as it really is there:
> >
> > $ ./bin/mips64el-linux-readelf -a ./mips64el-buildroot-linux-
> uclibc/sysroot/lib/libgcc_s.so.1 | grep multi3
> >   1747: 00011700   100 FUNC    GLOBAL DEFAULT   11 __multi3@@GCC_3.0
> >   5511: 00011700   100 FUNC    GLOBAL DEFAULT   11 __multi3
> >   000435e4 -32236(gp) 00011700 00011700 FUNC     11 __multi3
> >
> > Objdump says:
> >
> > 00011700 <__multi3>:
> >    11700:       0006103e        dsrl32  v0,a2,0x0
> >    11704:       7c89f803        dext    a5,a0,0x0,0x20
> >    11708:       0004403e        dsrl32  a4,a0,0x0
> >    1170c:       7ccaf803        dext    a6,a2,0x0,0x20
> >    11710:       012a589c        dmul    a7,a5,a6
> >    11714:       010a509c        dmul    a6,a4,a6
> >    11718:       0122489c        dmul    a5,a5,v0
> >    1171c:       0102409c        dmul    a4,a4,v0
> >    11720:       012a482d        daddu   a5,a5,a6
> >    11724:       000b103e        dsrl32  v0,a7,0x0
> >    11728:       0049102d        daddu   v0,v0,a5
> >    1172c:       184a0003        bgeuc   v0,a6,1173c <.L2>
> >    11730:       24090001        li      a5,1
> >    11734:       0009483c        dsll32  a5,a5,0x0
> >    11738:       0109402d        daddu   a4,a4,a5
> 
> I happened to have a GCC build dir around so I greped for __multi3 and
> found it hiding in muldi3.o.
> 
> Maybe that'obvious for those in the know, not me :)  Also that .o file
> contained MIPS III code and I was able to get GCC to emit a reference
> to __multi3 for MIPS III or MIPS64R1 targets, so version of __multi3
> even seems unused.

Sorry for the long delay in replying.

I think this could be considered a GCC bug. The multiply pattern support
in MIPS GCC is incredibly complex but the net effect is supposed to be
a guarantee that certain multiply operations will never need to use
helpers and as such we don't provide the helpers in those cases. There
are however special cases of course. The widening multiply from DImode
to TImode can't be generated by the compiler when working around R4000
errata and the extension is unsigned from DImode to TImode. I don't
believe TImode multiplies (i.e. full width 128-bit multiplies) are
generated from MIPS GCC but I don't know whether we have sleep-walked
into having __int128 support which would muddy the water somewhat.

So... In an R6 build I don't think the __multi3 helper should be generated
and I think the offending pattern needs extending for R6 support:

(define_expand "<u>mulditi3"
  [(set (match_operand:TI 0 "register_operand")
        (mult:TI (any_extend:TI (match_operand:DI 1 "register_operand"))
                 (any_extend:TI (match_operand:DI 2 "register_operand"))))]
  "ISA_HAS_DMULT && !(<CODE> == ZERO_EXTEND && TARGET_FIX_VR4120)"
{
  rtx hilo;

  if (TARGET_MIPS16)
    {
      hilo = gen_rtx_REG (TImode, MD_REG_FIRST);
      emit_insn (gen_<u>mulditi3_internal (hilo, operands[1], operands[2]));
      emit_move_insn (operands[0], hilo);
    }
  else if (TARGET_FIX_R4000)
    emit_insn (gen_<u>mulditi3_r4000 (operands[0], operands[1], operands[2]));
  else
    emit_insn (gen_<u>mulditi3_internal (operands[0], operands[1],
                                         operands[2]));
  DONE;
})

For SI->DI mode multiplies on 32bit R6 we have the following which naturally
ports to DI->TI:

(define_expand "<u>mulsidi3_32bit_r6"
  [(set (match_operand:DI 0 "register_operand")
        (mult:DI (any_extend:DI (match_operand:SI 1 "register_operand"))
                 (any_extend:DI (match_operand:SI 2 "register_operand"))))]
  "!TARGET_64BIT && ISA_HAS_R6MUL"
{
  rtx dest = gen_reg_rtx (DImode);
  rtx low = mips_subword (dest, 0);
  rtx high = mips_subword (dest, 1);

  emit_insn (gen_mulsi3_mul3_nohilo (low, operands[1], operands[2]));
  emit_insn (gen_<su>mulsi3_highpart_r6 (high, operands[1], operands[2]));

  emit_move_insn (mips_subword (operands[0], 0), low);
  emit_move_insn (mips_subword (operands[0], 1), high);
  DONE;
})

Despite the theory being simple, wiring this up will take time as it also
involves getting the costing calculations updated.

Please can you submit it as a GCC bug?

As a workaround you may want to include a version of __multi3 in the kernel
until it is resolved.

Thanks,
Matthew

Re: undefined reference to `__multi3' when building with gcc 7.x

[Ralf Baechle]

On Thu, Aug 17, 2017 at 08:49:13AM +0000, Matthew Fortune wrote:
> Date:   Thu, 17 Aug 2017 08:49:13 +0000
> From: Matthew Fortune <Matthew.Fortune@imgtec.com>
> To: Ralf Baechle <ralf@linux-mips.org>, Thomas Petazzoni
>  <thomas.petazzoni@free-electrons.com>
> CC: "linux-mips@linux-mips.org" <linux-mips@linux-mips.org>, Waldemar
>  Brodkorb <wbx@openadk.org>
> Subject: RE: undefined reference to `__multi3' when building with gcc 7.x
> Content-Type: text/plain; charset="us-ascii"
> 
> Ralf Baechle <ralf@linux-mips.org> writes:
> > On Sun, Aug 13, 2017 at 10:46:02PM +0200, Thomas Petazzoni wrote:
> > > Date:   Sun, 13 Aug 2017 22:46:02 +0200
> > > From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > > To: Ralf Baechle <ralf@linux-mips.org>
> > > Cc: Matthew Fortune <Matthew.Fortune@imgtec.com>,
> > >  linux-mips@linux-mips.org, Waldemar Brodkorb <wbx@openadk.org>
> > > Subject: Re: undefined reference to `__multi3' when building with gcc 7.x
> > > Content-Type: text/plain; charset=US-ASCII
> > >
> > > Hello,
> > >
> > > On Mon, 7 Aug 2017 10:34:48 +0200, Ralf Baechle wrote:
> > >
> > > > > > Chances are it's something specific to MIPS64 R6.  Before trying your
> > > > > > config file I also tried a number of other defconfigs and all built
> > > > > > well.
> > > > > >
> > > > > > Here's a test case which generates a reference to __multi3:
> > > > > >
> > > > > > unsigned long func(unsigned long a, unsigned long b)
> > > > > > {
> > > > > >         return a > (~0UL) / b;
> > > > > > }
> > > > > >
> > > > > > GCC rearanges above statement to:
> > > > > >
> > > > > > 	return (unsigned __int128)a * (unsigned __int128) b > 0xffffffff;
> > > > >
> > > > > And this is normal/expected ?
> > > >
> > > > Without consideration of performance, It's certainly is valid code.  And
> > > > with that I can't drop the issue as a GCC code generation bug.
> > > >
> > > > However it seems GCC itself doesn't seem to have a __multi3 in its
> > > > libgcc2 - which indeed would be a GCC issue - at least none I was easily
> > > > able to find with grep so I'm adding Matthew Fortune to cc in the hope he
> > > > can shed some light on this.
> > >
> > > Indeed, I don't see __multi3 implemented in libgcc in the source code,
> > > but it's probably because it's tricky to see its implementation,
> > > as it really is there:
> > >
> > > $ ./bin/mips64el-linux-readelf -a ./mips64el-buildroot-linux-
> > uclibc/sysroot/lib/libgcc_s.so.1 | grep multi3
> > >   1747: 00011700   100 FUNC    GLOBAL DEFAULT   11 __multi3@@GCC_3.0
> > >   5511: 00011700   100 FUNC    GLOBAL DEFAULT   11 __multi3
> > >   000435e4 -32236(gp) 00011700 00011700 FUNC     11 __multi3
> > >
> > > Objdump says:
> > >
> > > 00011700 <__multi3>:
> > >    11700:       0006103e        dsrl32  v0,a2,0x0
> > >    11704:       7c89f803        dext    a5,a0,0x0,0x20
> > >    11708:       0004403e        dsrl32  a4,a0,0x0
> > >    1170c:       7ccaf803        dext    a6,a2,0x0,0x20
> > >    11710:       012a589c        dmul    a7,a5,a6
> > >    11714:       010a509c        dmul    a6,a4,a6
> > >    11718:       0122489c        dmul    a5,a5,v0
> > >    1171c:       0102409c        dmul    a4,a4,v0
> > >    11720:       012a482d        daddu   a5,a5,a6
> > >    11724:       000b103e        dsrl32  v0,a7,0x0
> > >    11728:       0049102d        daddu   v0,v0,a5
> > >    1172c:       184a0003        bgeuc   v0,a6,1173c <.L2>
> > >    11730:       24090001        li      a5,1
> > >    11734:       0009483c        dsll32  a5,a5,0x0
> > >    11738:       0109402d        daddu   a4,a4,a5
> > 
> > I happened to have a GCC build dir around so I greped for __multi3 and
> > found it hiding in muldi3.o.
> > 
> > Maybe that'obvious for those in the know, not me :)  Also that .o file
> > contained MIPS III code and I was able to get GCC to emit a reference
> > to __multi3 for MIPS III or MIPS64R1 targets, so version of __multi3
> > even seems unused.
> 
> Sorry for the long delay in replying.
> 
> I think this could be considered a GCC bug. The multiply pattern support
> in MIPS GCC is incredibly complex but the net effect is supposed to be
> a guarantee that certain multiply operations will never need to use
> helpers and as such we don't provide the helpers in those cases. There
> are however special cases of course. The widening multiply from DImode
> to TImode can't be generated by the compiler when working around R4000
> errata and the extension is unsigned from DImode to TImode. I don't
> believe TImode multiplies (i.e. full width 128-bit multiplies) are
> generated from MIPS GCC but I don't know whether we have sleep-walked
> into having __int128 support which would muddy the water somewhat.
> 
> So... In an R6 build I don't think the __multi3 helper should be generated
> and I think the offending pattern needs extending for R6 support:
> 
> (define_expand "<u>mulditi3"
>   [(set (match_operand:TI 0 "register_operand")
>         (mult:TI (any_extend:TI (match_operand:DI 1 "register_operand"))
>                  (any_extend:TI (match_operand:DI 2 "register_operand"))))]
>   "ISA_HAS_DMULT && !(<CODE> == ZERO_EXTEND && TARGET_FIX_VR4120)"
> {
>   rtx hilo;
> 
>   if (TARGET_MIPS16)
>     {
>       hilo = gen_rtx_REG (TImode, MD_REG_FIRST);
>       emit_insn (gen_<u>mulditi3_internal (hilo, operands[1], operands[2]));
>       emit_move_insn (operands[0], hilo);
>     }
>   else if (TARGET_FIX_R4000)
>     emit_insn (gen_<u>mulditi3_r4000 (operands[0], operands[1], operands[2]));
>   else
>     emit_insn (gen_<u>mulditi3_internal (operands[0], operands[1],
>                                          operands[2]));
>   DONE;
> })
> 
> For SI->DI mode multiplies on 32bit R6 we have the following which naturally
> ports to DI->TI:
> 
> (define_expand "<u>mulsidi3_32bit_r6"
>   [(set (match_operand:DI 0 "register_operand")
>         (mult:DI (any_extend:DI (match_operand:SI 1 "register_operand"))
>                  (any_extend:DI (match_operand:SI 2 "register_operand"))))]
>   "!TARGET_64BIT && ISA_HAS_R6MUL"
> {
>   rtx dest = gen_reg_rtx (DImode);
>   rtx low = mips_subword (dest, 0);
>   rtx high = mips_subword (dest, 1);
> 
>   emit_insn (gen_mulsi3_mul3_nohilo (low, operands[1], operands[2]));
>   emit_insn (gen_<su>mulsi3_highpart_r6 (high, operands[1], operands[2]));
> 
>   emit_move_insn (mips_subword (operands[0], 0), low);
>   emit_move_insn (mips_subword (operands[0], 1), high);
>   DONE;
> })
> 
> Despite the theory being simple, wiring this up will take time as it also
> involves getting the costing calculations updated.
> 
> Please can you submit it as a GCC bug?

Will do.

> As a workaround you may want to include a version of __multi3 in the kernel
> until it is resolved.

Yes, working on that.  This has been made harder by the fact that the
implementation of __umulti3 is well hidden in the source :)  I now have
functioning implementation of __multi3 but it's still too ugly to be
committed to the kernel.

And while I agree it should be fixed in GCC at the same time the
generated code while convoluted and unnecessarily slow appears to be
correct so I think we should support this by adding a suitable __umulti3
to the kernel code as you suggest.

  Ralf

Re: undefined reference to `__multi3' when building with gcc 7.x

[Thomas Petazzoni]

Hello,

On Fri, 18 Aug 2017 00:19:31 +0200, Ralf Baechle wrote:

> > Despite the theory being simple, wiring this up will take time as it also
> > involves getting the costing calculations updated.
> > 
> > Please can you submit it as a GCC bug?  
> 
> Will do.
> 
> > As a workaround you may want to include a version of __multi3 in the kernel
> > until it is resolved.  
> 
> Yes, working on that.  This has been made harder by the fact that the
> implementation of __umulti3 is well hidden in the source :)  I now have
> functioning implementation of __multi3 but it's still too ugly to be
> committed to the kernel.
> 
> And while I agree it should be fixed in GCC at the same time the
> generated code while convoluted and unnecessarily slow appears to be
> correct so I think we should support this by adding a suitable __umulti3
> to the kernel code as you suggest.

Has there been any progress on solving this issue ? Either on the GCC
side or the kernel side ?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: undefined reference to `__multi3' when building with gcc 7.x

[Thomas Petazzoni]

Hello,

On Fri, 18 Aug 2017 00:19:31 +0200, Ralf Baechle wrote:

> > As a workaround you may want to include a version of __multi3 in the kernel
> > until it is resolved.  
> 
> Yes, working on that.  This has been made harder by the fact that the
> implementation of __umulti3 is well hidden in the source :)  I now have
> functioning implementation of __multi3 but it's still too ugly to be
> committed to the kernel.
> 
> And while I agree it should be fixed in GCC at the same time the
> generated code while convoluted and unnecessarily slow appears to be
> correct so I think we should support this by adding a suitable __umulti3
> to the kernel code as you suggest.

Has there been some progress on this front? I'm willing to test patches
if needed.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: undefined reference to `__multi3' when building with gcc 7.x

[Maciej W. Rozycki]

On Fri, 4 Aug 2017, Ralf Baechle wrote:

> Chances are it's something specific to MIPS64 R6.  Before trying your
> config file I also tried a number of other defconfigs and all built
> well.
> 
> Here's a test case which generates a reference to __multi3:
> 
> unsigned long func(unsigned long a, unsigned long b)
> {
>         return a > (~0UL) / b;
> }
> 
> GCC rearanges above statement to:
> 
> 	return (unsigned __int128)a * (unsigned __int128) b > 0xffffffff;
> 
> computing which requires 128 bit intermediate results.  This is the
> code generated for MIPS64 R2:
> 
> 	dmultu	$4,$5
> 	mfhi	$2
> 	sltu	$2,$0,$2
> 
> And this is for R6:
> 
> 	move	$2,$4
> 	move	$7,$5			# $6/$7 contain the second op
> 	move	$6,$0
> 	move	$4,$0
> 	move	$5,$2			# $4/$5 contain the first op
> 	sd	$31,8($sp)
> 	balc	__multi3
> 	sltu	$2,$0,$2

 This looks silly.  Why doesn't GCC simply emit:

	dmuhu	$2,$4,$5
	sltu	$2,$0,$2

instead?  It's even shorter than the R2 equivalent.

  Maciej

Re: undefined reference to `__multi3' when building with gcc 7.x

[Maciej W. Rozycki]

On Fri, 4 Aug 2017, Ralf Baechle wrote:

> Chances are it's something specific to MIPS64 R6.  Before trying your
> config file I also tried a number of other defconfigs and all built
> well.
> 
> Here's a test case which generates a reference to __multi3:
> 
> unsigned long func(unsigned long a, unsigned long b)
> {
>         return a > (~0UL) / b;
> }
> 
> GCC rearanges above statement to:
> 
> 	return (unsigned __int128)a * (unsigned __int128) b > 0xffffffff;
> 
> computing which requires 128 bit intermediate results.  This is the
> code generated for MIPS64 R2:
> 
> 	dmultu	$4,$5
> 	mfhi	$2
> 	sltu	$2,$0,$2
> 
> And this is for R6:
> 
> 	move	$2,$4
> 	move	$7,$5			# $6/$7 contain the second op
> 	move	$6,$0
> 	move	$4,$0
> 	move	$5,$2			# $4/$5 contain the first op
> 	sd	$31,8($sp)
> 	balc	__multi3
> 	sltu	$2,$0,$2

 This looks silly.  Why doesn't GCC simply emit:

	dmuhu	$2,$4,$5
	sltu	$2,$0,$2

instead?  It's even shorter than the R2 equivalent.

  Maciej

Re: undefined reference to `__multi3' when building with gcc 7.x

[Thomas Petazzoni]

Hello,

+James Hogan in Cc.

On Thu, 3 Aug 2017 22:55:47 +0200, Thomas Petazzoni wrote:

> When trying to build the current Linux master with a gcc 7.x toolchain
> for mips64r6-n32, I'm getting the following build failure:
> 
> crypto/scompress.o: In function `.L31':
> scompress.c:(.text+0x2a0): undefined reference to `__multi3'
> drivers/base/component.o: In function `.L97':
> component.c:(.text+0x4a4): undefined reference to `__multi3'
> drivers/base/component.o: In function `component_master_add_with_match':
> component.c:(.text+0x8c4): undefined reference to `__multi3'
> net/core/ethtool.o: In function `ethtool_set_per_queue_coalesce':
> ethtool.c:(.text+0x1ab0): undefined reference to `__multi3'
> Makefile:1000: recipe for target 'vmlinux' failed
> make[2]: *** [vmlinux] Error 1

I'm still facing this problem. There was a lengthy thread about it back
in August when I reported the problem, but then it calmed down, with no
real solution proposed.

Are there plans to fix this at some point?

Thanks a lot,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: undefined reference to `__multi3' when building with gcc 7.x

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

On Sun, Dec 03, 2017 at 10:56:31AM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> +James Hogan in Cc.
> 
> On Thu, 3 Aug 2017 22:55:47 +0200, Thomas Petazzoni wrote:
> 
> > When trying to build the current Linux master with a gcc 7.x toolchain
> > for mips64r6-n32, I'm getting the following build failure:
> > 
> > crypto/scompress.o: In function `.L31':
> > scompress.c:(.text+0x2a0): undefined reference to `__multi3'
> > drivers/base/component.o: In function `.L97':
> > component.c:(.text+0x4a4): undefined reference to `__multi3'
> > drivers/base/component.o: In function `component_master_add_with_match':
> > component.c:(.text+0x8c4): undefined reference to `__multi3'
> > net/core/ethtool.o: In function `ethtool_set_per_queue_coalesce':
> > ethtool.c:(.text+0x1ab0): undefined reference to `__multi3'
> > Makefile:1000: recipe for target 'vmlinux' failed
> > make[2]: *** [vmlinux] Error 1
> 
> I'm still facing this problem. There was a lengthy thread about it back
> in August when I reported the problem, but then it calmed down, with no
> real solution proposed.
> 
> Are there plans to fix this at some point?

I recently fixed a similar issue in 64r6[el]_defconfig, but its not the
same as it applies to all gcc versions on mips64r6. Given Ralf appears
to be busy I'll take a look.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: undefined reference to `__multi3' when building with gcc 7.x

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

On Sun, Dec 03, 2017 at 10:56:31AM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> +James Hogan in Cc.
> 
> On Thu, 3 Aug 2017 22:55:47 +0200, Thomas Petazzoni wrote:
> 
> > When trying to build the current Linux master with a gcc 7.x toolchain
> > for mips64r6-n32, I'm getting the following build failure:
> > 
> > crypto/scompress.o: In function `.L31':
> > scompress.c:(.text+0x2a0): undefined reference to `__multi3'
> > drivers/base/component.o: In function `.L97':
> > component.c:(.text+0x4a4): undefined reference to `__multi3'
> > drivers/base/component.o: In function `component_master_add_with_match':
> > component.c:(.text+0x8c4): undefined reference to `__multi3'
> > net/core/ethtool.o: In function `ethtool_set_per_queue_coalesce':
> > ethtool.c:(.text+0x1ab0): undefined reference to `__multi3'
> > Makefile:1000: recipe for target 'vmlinux' failed
> > make[2]: *** [vmlinux] Error 1
> 
> I'm still facing this problem. There was a lengthy thread about it back
> in August when I reported the problem, but then it calmed down, with no
> real solution proposed.
> 
> Are there plans to fix this at some point?

I recently fixed a similar issue in 64r6[el]_defconfig, but its not the
same as it applies to all gcc versions on mips64r6. Given Ralf appears
to be busy I'll take a look.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: undefined reference to `__multi3' when building with gcc 7.x

[Thomas Petazzoni]

Hello,

On Tue, 5 Dec 2017 23:49:24 +0000, James Hogan wrote:

> > I'm still facing this problem. There was a lengthy thread about it back
> > in August when I reported the problem, but then it calmed down, with no
> > real solution proposed.
> > 
> > Are there plans to fix this at some point?  
> 
> I recently fixed a similar issue in 64r6[el]_defconfig, but its not the
> same as it applies to all gcc versions on mips64r6. Given Ralf appears
> to be busy I'll take a look.

Thanks a lot. If you need some help to reproduce the problem, let me
know, I can provide simple instructions to produce it.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: undefined reference to `__multi3' when building with gcc 7.x

[Thomas Petazzoni]

Hello,

On Tue, 5 Dec 2017 23:49:24 +0000, James Hogan wrote:

> > I'm still facing this problem. There was a lengthy thread about it back
> > in August when I reported the problem, but then it calmed down, with no
> > real solution proposed.
> > 
> > Are there plans to fix this at some point?  
> 
> I recently fixed a similar issue in 64r6[el]_defconfig, but its not the
> same as it applies to all gcc versions on mips64r6. Given Ralf appears
> to be busy I'll take a look.

Thanks a lot. If you need some help to reproduce the problem, let me
know, I can provide simple instructions to produce it.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH] MIPS: Implement __multi3 for GCC7 MIPS64r6 builds

[James Hogan]

GCC7 is a bit too eager to generate suboptimal __multi3 calls (128bit
multiply with 128bit result) for MIPS64r6 builds, even in code which
doesn't explicitly use 128bit types, such as the following:

unsigned long func(unsigned long a, unsigned long b)
{
	return a > (~0UL) / b;
}

Which GCC rearanges to:

return (unsigned __int128)a * (unsigned __int128)b > 0xffffffff;

Therefore implement __multi3, but only for MIPS64r6 with GCC7 as under
normal circumstances we wouldn't expect any calls to __multi3 to be
generated from kernel code.

Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: James Hogan <jhogan@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Maciej W. Rozycki <macro@mips.com>
Cc: Matthew Fortune <matthew.fortune@mips.com>
Cc: Florian Fainelli <florian@openwrt.org>
Cc: Waldemar Brodkorb <wbx@openadk.org>
Cc: linux-mips@linux-mips.org
---
This should fix the issue Thomas. Thanks for reporting.
---
 arch/mips/lib/Makefile |  3 ++-
 arch/mips/lib/libgcc.h | 17 +++++++++++++++++
 arch/mips/lib/multi3.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 arch/mips/lib/multi3.c

diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
index 78c2affeabf8..e84e12655fa8 100644
--- a/arch/mips/lib/Makefile
+++ b/arch/mips/lib/Makefile
@@ -16,4 +16,5 @@ obj-$(CONFIG_CPU_R3000)		+= r3k_dump_tlb.o
 obj-$(CONFIG_CPU_TX39XX)	+= r3k_dump_tlb.o
 
 # libgcc-style stuff needed in the kernel
-obj-y += ashldi3.o ashrdi3.o bswapsi.o bswapdi.o cmpdi2.o lshrdi3.o ucmpdi2.o
+obj-y += ashldi3.o ashrdi3.o bswapsi.o bswapdi.o cmpdi2.o lshrdi3.o multi3.o \
+	 ucmpdi2.o
diff --git a/arch/mips/lib/libgcc.h b/arch/mips/lib/libgcc.h
index 28002ed90c2c..199a7f96282f 100644
--- a/arch/mips/lib/libgcc.h
+++ b/arch/mips/lib/libgcc.h
@@ -10,10 +10,18 @@ typedef int word_type __attribute__ ((mode (__word__)));
 struct DWstruct {
 	int high, low;
 };
+
+struct TWstruct {
+	long long high, low;
+};
 #elif defined(__LITTLE_ENDIAN)
 struct DWstruct {
 	int low, high;
 };
+
+struct TWstruct {
+	long long low, high;
+};
 #else
 #error I feel sick.
 #endif
@@ -23,4 +31,13 @@ typedef union {
 	long long ll;
 } DWunion;
 
+#if defined(CONFIG_64BIT) && defined(CONFIG_CPU_MIPSR6)
+typedef int ti_type __attribute__((mode(TI)));
+
+typedef union {
+	struct TWstruct s;
+	ti_type ti;
+} TWunion;
+#endif
+
 #endif /* __ASM_LIBGCC_H */
diff --git a/arch/mips/lib/multi3.c b/arch/mips/lib/multi3.c
new file mode 100644
index 000000000000..fba123e366c8
--- /dev/null
+++ b/arch/mips/lib/multi3.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/export.h>
+
+#include "libgcc.h"
+
+/*
+ * GCC 7 suboptimally generates __multi3 calls for mips64r6, so for that
+ * specific case only we'll implement it here.
+ *
+ * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82981
+ */
+#if defined(CONFIG_64BIT) && defined(CONFIG_CPU_MIPSR6) && (__GNUC__ == 7)
+
+/* multiply 64-bit values, low 64-bits returned */
+static inline long long notrace dmulu(long long a, long long b)
+{
+	long long res;
+	asm ("dmulu %0,%1,%2" : "=r" (res) : "r" (a), "r" (b));
+	return res;
+}
+
+/* multiply 64-bit unsigned values, high 64-bits of 128-bit result returned */
+static inline long long notrace dmuhu(long long a, long long b)
+{
+	long long res;
+	asm ("dmuhu %0,%1,%2" : "=r" (res) : "r" (a), "r" (b));
+	return res;
+}
+
+/* multiply 128-bit values, low 128-bits returned */
+ti_type notrace __multi3(ti_type a, ti_type b)
+{
+	TWunion res, aa, bb;
+
+	aa.ti = a;
+	bb.ti = b;
+
+	/*
+	 * a * b =           (a.lo * b.lo)
+	 *         + 2^64  * (a.hi * b.lo + a.lo * b.hi)
+	 *        [+ 2^128 * (a.hi * b.hi)]
+	 */
+	res.s.low = dmulu(aa.s.low, bb.s.low);
+	res.s.high = dmuhu(aa.s.low, bb.s.low);
+	res.s.high += dmulu(aa.s.high, bb.s.low);
+	res.s.high += dmulu(aa.s.low, bb.s.high);
+
+	return res.ti;
+}
+EXPORT_SYMBOL(__multi3);
+
+#endif /* 64BIT && CPU_MIPSR6 && GCC7 */
-- 
2.13.6

Re: [PATCH] MIPS: Implement __multi3 for GCC7 MIPS64r6 builds

[Maciej W. Rozycki]

On Wed, 6 Dec 2017, James Hogan wrote:

> GCC7 is a bit too eager to generate suboptimal __multi3 calls (128bit
> multiply with 128bit result) for MIPS64r6 builds, even in code which
> doesn't explicitly use 128bit types, such as the following:
> 
> unsigned long func(unsigned long a, unsigned long b)
> {
> 	return a > (~0UL) / b;
> }
> 
> Which GCC rearanges to:
> 
> return (unsigned __int128)a * (unsigned __int128)b > 0xffffffff;

 You mean:

return (unsigned __int128)a * (unsigned __int128)b > 0xffffffffffffffff;

presumably, or is there another bug here?

> Therefore implement __multi3, but only for MIPS64r6 with GCC7 as under
> normal circumstances we wouldn't expect any calls to __multi3 to be
> generated from kernel code.

 That does look bad; I'd expect a `umulditi3' (widening 64-bit by 64-bit 
unsigned multiplication) kind of operation instead, which should expand 
internally.  And we only really need to execute DMUHU and then check the 
result for non-zero here, because the value of the low 64 bits of the 
product does not matter for the evaluation of the expression.

 I don't know offhand if such a transformation can be handled by GCC as it 
stands by tweaking the MIPS backend without a corresponding update to the 
middle end though.

  Maciej

Re: [PATCH] MIPS: Implement __multi3 for GCC7 MIPS64r6 builds

[Maciej W. Rozycki]

On Wed, 6 Dec 2017, James Hogan wrote:

> GCC7 is a bit too eager to generate suboptimal __multi3 calls (128bit
> multiply with 128bit result) for MIPS64r6 builds, even in code which
> doesn't explicitly use 128bit types, such as the following:
> 
> unsigned long func(unsigned long a, unsigned long b)
> {
> 	return a > (~0UL) / b;
> }
> 
> Which GCC rearanges to:
> 
> return (unsigned __int128)a * (unsigned __int128)b > 0xffffffff;

 You mean:

return (unsigned __int128)a * (unsigned __int128)b > 0xffffffffffffffff;

presumably, or is there another bug here?

> Therefore implement __multi3, but only for MIPS64r6 with GCC7 as under
> normal circumstances we wouldn't expect any calls to __multi3 to be
> generated from kernel code.

 That does look bad; I'd expect a `umulditi3' (widening 64-bit by 64-bit 
unsigned multiplication) kind of operation instead, which should expand 
internally.  And we only really need to execute DMUHU and then check the 
result for non-zero here, because the value of the low 64 bits of the 
product does not matter for the evaluation of the expression.

 I don't know offhand if such a transformation can be handled by GCC as it 
stands by tweaking the MIPS backend without a corresponding update to the 
middle end though.

  Maciej

Re: [PATCH] MIPS: Implement __multi3 for GCC7 MIPS64r6 builds

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]

On Fri, Dec 08, 2017 at 11:52:05PM +0000, Maciej W. Rozycki wrote:
> On Wed, 6 Dec 2017, James Hogan wrote:
> 
> > GCC7 is a bit too eager to generate suboptimal __multi3 calls (128bit
> > multiply with 128bit result) for MIPS64r6 builds, even in code which
> > doesn't explicitly use 128bit types, such as the following:
> > 
> > unsigned long func(unsigned long a, unsigned long b)
> > {
> > 	return a > (~0UL) / b;
> > }
> > 
> > Which GCC rearanges to:
> > 
> > return (unsigned __int128)a * (unsigned __int128)b > 0xffffffff;
> 
>  You mean:
> 
> return (unsigned __int128)a * (unsigned __int128)b > 0xffffffffffffffff;
> 
> presumably, or is there another bug here?

Yes, thats what was meant. It was copy + pasted from Ralf's analysis.

Thanks
James

> 
> > Therefore implement __multi3, but only for MIPS64r6 with GCC7 as under
> > normal circumstances we wouldn't expect any calls to __multi3 to be
> > generated from kernel code.
> 
>  That does look bad; I'd expect a `umulditi3' (widening 64-bit by 64-bit 
> unsigned multiplication) kind of operation instead, which should expand 
> internally.  And we only really need to execute DMUHU and then check the 
> result for non-zero here, because the value of the low 64 bits of the 
> product does not matter for the evaluation of the expression.
> 
>  I don't know offhand if such a transformation can be handled by GCC as it 
> stands by tweaking the MIPS backend without a corresponding update to the 
> middle end though.
> 
>   Maciej
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] MIPS: Implement __multi3 for GCC7 MIPS64r6 builds

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]

On Fri, Dec 08, 2017 at 11:52:05PM +0000, Maciej W. Rozycki wrote:
> On Wed, 6 Dec 2017, James Hogan wrote:
> 
> > GCC7 is a bit too eager to generate suboptimal __multi3 calls (128bit
> > multiply with 128bit result) for MIPS64r6 builds, even in code which
> > doesn't explicitly use 128bit types, such as the following:
> > 
> > unsigned long func(unsigned long a, unsigned long b)
> > {
> > 	return a > (~0UL) / b;
> > }
> > 
> > Which GCC rearanges to:
> > 
> > return (unsigned __int128)a * (unsigned __int128)b > 0xffffffff;
> 
>  You mean:
> 
> return (unsigned __int128)a * (unsigned __int128)b > 0xffffffffffffffff;
> 
> presumably, or is there another bug here?

Yes, thats what was meant. It was copy + pasted from Ralf's analysis.

Thanks
James

> 
> > Therefore implement __multi3, but only for MIPS64r6 with GCC7 as under
> > normal circumstances we wouldn't expect any calls to __multi3 to be
> > generated from kernel code.
> 
>  That does look bad; I'd expect a `umulditi3' (widening 64-bit by 64-bit 
> unsigned multiplication) kind of operation instead, which should expand 
> internally.  And we only really need to execute DMUHU and then check the 
> result for non-zero here, because the value of the low 64 bits of the 
> product does not matter for the evaluation of the expression.
> 
>  I don't know offhand if such a transformation can be handled by GCC as it 
> stands by tweaking the MIPS backend without a corresponding update to the 
> middle end though.
> 
>   Maciej
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] MIPS: Implement __multi3 for GCC7 MIPS64r6 builds

[Waldemar Brodkorb]

Hi James,
James Hogan wrote,

> GCC7 is a bit too eager to generate suboptimal __multi3 calls (128bit
> multiply with 128bit result) for MIPS64r6 builds, even in code which
> doesn't explicitly use 128bit types, such as the following:
> 
> unsigned long func(unsigned long a, unsigned long b)
> {
> 	return a > (~0UL) / b;
> }
> 
> Which GCC rearanges to:
> 
> return (unsigned __int128)a * (unsigned __int128)b > 0xffffffff;
> 
> Therefore implement __multi3, but only for MIPS64r6 with GCC7 as under
> normal circumstances we wouldn't expect any calls to __multi3 to be
> generated from kernel code.

I tested the patch and it works fine for me. I can build a mips64r6
kernel and run the uClibc-ng testsuite inside qemu system emulation.

It works for me with 4.9.x and 4.14.x kernels.

Thanks
 Waldemar