Re: About __udivdi3

Re: About __udivdi3

[Eric Lin]

Cc: mailing list
On Mon, Aug 05, 2019 at 01:58:56PM +0800, Eric Lin wrote:
> Hi Palmer/Paul, 
> 
> Recently I encounter a problem that the output from __udivdi3 is wrong.
> The compiler will generate __udivdi3 library call when doing 64-bit
> over 64-bit division on 32-bit platform.
> 
> It seems __udivdi3 only supports 64-bit platform.
> Do you have any comments? Thanks.
> 
> Best Regards,
> Eric Lin  

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: About __udivdi3

[Paul Walmsley]

On Tue, 6 Aug 2019, Eric Lin wrote:

> Cc: mailing list
> On Mon, Aug 05, 2019 at 01:58:56PM +0800, Eric Lin wrote:
> > Hi Palmer/Paul, 
> > 
> > Recently I encounter a problem that the output from __udivdi3 is wrong.
> > The compiler will generate __udivdi3 library call when doing 64-bit
> > over 64-bit division on 32-bit platform.
> > 
> > It seems __udivdi3 only supports 64-bit platform.
> > Do you have any comments? Thanks.

It sounds like you're describing a potential issue with gcc, rather than 
the kernel.  If that's so, then the gcc mailing list(s) would be a better 
place to discuss this:  https://gcc.gnu.org/lists.html
 

- Paul

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: About __udivdi3

[Eric Lin]

Hi Paul,

On Tue, Aug 06, 2019 at 10:13:40AM +0800, Paul Walmsley wrote:
> On Tue, 6 Aug 2019, Eric Lin wrote:
> 
> > Cc: mailing list
> > On Mon, Aug 05, 2019 at 01:58:56PM +0800, Eric Lin wrote:
> > > Hi Palmer/Paul, 
> > > 
> > > Recently I encounter a problem that the output from __udivdi3 is wrong.
> > > The compiler will generate __udivdi3 library call when doing 64-bit
> > > over 64-bit division on 32-bit platform.
> > > 
> > > It seems __udivdi3 only supports 64-bit platform.
> > > Do you have any comments? Thanks.
> 
> It sounds like you're describing a potential issue with gcc, rather than 
> the kernel.  If that's so, then the gcc mailing list(s) would be a better 
> place to discuss this:  https://gcc.gnu.org/lists.html
>  
> 
> - Paul

Sorry, maybe I didn't describe it clearly.
When doing 64-bit over 64-bit division on 32-bit platform, the gcc will
link the __udivdi3 function in arch/riscv/lib/udivdi3.S, but the output
is wrong because the function is designed for RV64 ABI only.

I've tried to modify the arch/riscv/Makefile to make it link libgcc's
__udivdi3 library. The output is correct after using the libgcc's __udivdi3.

So, I don't think the __udivdi3 in arch/riscv/lib can support 64-bit
over 64-bit division on 32-bit platform.

Best Regards,
Eric

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: About __udivdi3

[Jim Wilson]

On Mon, Aug 5, 2019 at 7:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
> > > It seems __udivdi3 only supports 64-bit platform.
> > > Do you have any comments? Thanks.
>
> It sounds like you're describing a potential issue with gcc, rather than
> the kernel.  If that's so, then the gcc mailing list(s) would be a better
> place to discuss this:  https://gcc.gnu.org/lists.html

The kernel doesn't link with libgcc.a.  It has its own copy of udivdi3
when needed.

Jim

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: About __udivdi3

[Nicolas Pitre]

On Mon, 5 Aug 2019, Jim Wilson wrote:

> On Mon, Aug 5, 2019 at 7:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
> > > > It seems __udivdi3 only supports 64-bit platform.
> > > > Do you have any comments? Thanks.
> >
> > It sounds like you're describing a potential issue with gcc, rather than
> > the kernel.  If that's so, then the gcc mailing list(s) would be a better
> > place to discuss this:  https://gcc.gnu.org/lists.html
> 
> The kernel doesn't link with libgcc.a.  It has its own copy of udivdi3
> when needed.

Looking at he udivdi3 code in the kernel, it certainly looks to me that 
it is meant for RV64 compilation i.e. 64-bit-wide registers. It won't 
produce the right result for sure if compiled for RV32.

That being said, a 32-bit kernel should never require udivdi3. The other 
32-bit architectures expressly omitted it on purpose to prevent its use. 
Most of the time, what is needed is a 64 by 32 division and, 
unfortunately, gcc doesn't know how to optimize that on 32-bit 
platforms. This is what the do_div() macro is for.

So the "lib-$(CONFIG_32BIT) += udivdi3.o" in arch/riscv/lib/Makefile is 
completely wrong. I could see its presence if the div instruction isn't 
available on some RV64 implementation, but that should be removed 
otherwise.


Nicolas

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: About __udivdi3

[Christoph Hellwig]

On Mon, Aug 05, 2019 at 08:01:27PM -0700, Jim Wilson wrote:
> On Mon, Aug 5, 2019 at 7:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
> > > > It seems __udivdi3 only supports 64-bit platform.
> > > > Do you have any comments? Thanks.
> >
> > It sounds like you're describing a potential issue with gcc, rather than
> > the kernel.  If that's so, then the gcc mailing list(s) would be a better
> > place to discuss this:  https://gcc.gnu.org/lists.html
> 
> The kernel doesn't link with libgcc.a.  It has its own copy of udivdi3
> when needed.

Linus still maintains his stance to not support 64 by 64-bit divisions
on 32-bit platforms, and thus Linux does not support it.  You can do
64 by 32-bit divisions using the awkward do_div macros, but that's it.

This is supposed to apply to all 32-bit platforms, not just RISC-V.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: About __udivdi3

[Palmer Dabbelt]

On Tue, 06 Aug 2019 01:31:35 PDT (-0700), Christoph Hellwig wrote:
> On Mon, Aug 05, 2019 at 08:01:27PM -0700, Jim Wilson wrote:
>> On Mon, Aug 5, 2019 at 7:13 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
>> > > > It seems __udivdi3 only supports 64-bit platform.
>> > > > Do you have any comments? Thanks.
>> >
>> > It sounds like you're describing a potential issue with gcc, rather than
>> > the kernel.  If that's so, then the gcc mailing list(s) would be a better
>> > place to discuss this:  https://gcc.gnu.org/lists.html
>>
>> The kernel doesn't link with libgcc.a.  It has its own copy of udivdi3
>> when needed.
>
> Linus still maintains his stance to not support 64 by 64-bit divisions
> on 32-bit platforms, and thus Linux does not support it.  You can do
> 64 by 32-bit divisions using the awkward do_div macros, but that's it.
>
> This is supposed to apply to all 32-bit platforms, not just RISC-V.

Yep.  I think the best bet is to just drop it, with something like this:

    commit f7f2185e0e3e60a8b77fb519a98b752242ade235
    gpg: Signature made Tue 06 Aug 2019 08:06:30 AM PDT
    gpg:                using RSA key 00CE76D1834960DFCE886DF8EF4CA1502CCBAB41
    gpg:                issuer "palmer@dabbelt.com"
    gpg: Good signature from "Palmer Dabbelt <palmer@dabbelt.com>" [ultimate]
    gpg:                 aka "Palmer Dabbelt <palmer@sifive.com>" [ultimate]
    Author: Palmer Dabbelt <palmer@sifive.com>
    Date:   Tue Aug 6 08:03:59 2019 -0700
    
        RISC-V: Remove udivdi3
    
        This should never have landed in the first place: it was added as part
        of 64-bit divide support for 32-bit systems, but the kernel doesn't
        allow this sort of division.  I must have forgotten to remove it.
    
        This patch removes the support.  Since this routine only worked on
        64-bit platforms but was only built on 32-bit platforms, it's
        essentially just nonsense anyway.
    
        Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
    
    diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
    index 8e364ebf37de..267feaa10f6a 100644
    --- a/arch/riscv/lib/Makefile
    +++ b/arch/riscv/lib/Makefile
    @@ -5,5 +5,3 @@ lib-y	+= memset.o
     lib-y	+= uaccess.o
    
     lib-$(CONFIG_64BIT) += tishift.o
    -
    -lib-$(CONFIG_32BIT) += udivdi3.o
    diff --git a/arch/riscv/lib/udivdi3.S b/arch/riscv/lib/udivdi3.S
    deleted file mode 100644
    index 3f07476a91a9..000000000000
    --- a/arch/riscv/lib/udivdi3.S
    +++ /dev/null
    @@ -1,32 +0,0 @@
    -/* SPDX-License-Identifier: GPL-2.0-only */
    -/*
    - * Copyright (C) 2016-2017 Free Software Foundation, Inc.
    - */
    -
    -#include <linux/linkage.h>
    -
    -ENTRY(__udivdi3)
    -	mv	a2, a1
    -	mv	a1, a0
    -	li	a0, -1
    -	beqz	a2, .L5
    -	li	a3, 1
    -	bgeu	a2, a1, .L2
    -.L1:
    -	blez	a2, .L2
    -	slli	a2, a2, 1
    -	slli	a3, a3, 1
    -	bgtu	a1, a2, .L1
    -.L2:
    -	li	a0, 0
    -.L3:
    -	bltu	a1, a2, .L4
    -	sub	a1, a1, a2
    -	or	a0, a0, a3
    -.L4:
    -	srli	a3, a3, 1
    -	srli	a2, a2, 1
    -	bnez	a3, .L3
    -.L5:
    -	ret
    -ENDPROC(__udivdi3)
    

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: About __udivdi3

[Paul Walmsley]

(privately)

On Tue, 6 Aug 2019, Palmer Dabbelt wrote:

> Yep.  I think the best bet is to just drop it, with something like this:
> 
>    commit f7f2185e0e3e60a8b77fb519a98b752242ade235
>    gpg: Signature made Tue 06 Aug 2019 08:06:30 AM PDT
>    gpg:                using RSA key 00CE76D1834960DFCE886DF8EF4CA1502CCBAB41
>    gpg:                issuer "palmer@dabbelt.com"
>    gpg: Good signature from "Palmer Dabbelt <palmer@dabbelt.com>" [ultimate]
>    gpg:                 aka "Palmer Dabbelt <palmer@sifive.com>" [ultimate]
>    Author: Palmer Dabbelt <palmer@sifive.com>
>    Date:   Tue Aug 6 08:03:59 2019 -0700
>           RISC-V: Remove udivdi3
>           This should never have landed in the first place: it was added as
> part
>        of 64-bit divide support for 32-bit systems, but the kernel doesn't
>        allow this sort of division.  I must have forgotten to remove it.

Just FYI, you probably already know this, but this patch was mangled on 
the list.  I'll reformat it and queue it up, but figured you probably 
would want to know if you didn't already.


- Paul

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: About __udivdi3

[Palmer Dabbelt]

On Tue, 06 Aug 2019 10:53:12 PDT (-0700), Paul Walmsley wrote:
> (privately)
>
> On Tue, 6 Aug 2019, Palmer Dabbelt wrote:
>
>> Yep.  I think the best bet is to just drop it, with something like this:
>>
>>    commit f7f2185e0e3e60a8b77fb519a98b752242ade235
>>    gpg: Signature made Tue 06 Aug 2019 08:06:30 AM PDT
>>    gpg:                using RSA key 00CE76D1834960DFCE886DF8EF4CA1502CCBAB41
>>    gpg:                issuer "palmer@dabbelt.com"
>>    gpg: Good signature from "Palmer Dabbelt <palmer@dabbelt.com>" [ultimate]
>>    gpg:                 aka "Palmer Dabbelt <palmer@sifive.com>" [ultimate]
>>    Author: Palmer Dabbelt <palmer@sifive.com>
>>    Date:   Tue Aug 6 08:03:59 2019 -0700
>>           RISC-V: Remove udivdi3
>>           This should never have landed in the first place: it was added as
>> part
>>        of 64-bit divide support for 32-bit systems, but the kernel doesn't
>>        allow this sort of division.  I must have forgotten to remove it.
>
> Just FYI, you probably already know this, but this patch was mangled on
> the list.  I'll reformat it and queue it up, but figured you probably
> would want to know if you didn't already.

Ya, they're mangled when I inline them.  I'm usually just asking for people to
look at the concept here, and I send a real patch out later.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: About __udivdi3

[Nicolas Pitre]

On Tue, 6 Aug 2019, Palmer Dabbelt wrote:

>    Author: Palmer Dabbelt <palmer@sifive.com>
>    Date:   Tue Aug 6 08:03:59 2019 -0700
>    
>        RISC-V: Remove udivdi3
>    
>        This should never have landed in the first place: it was added as part
>        of 64-bit divide support for 32-bit systems, but the kernel doesn't
>        allow this sort of division.  I must have forgotten to remove it.
>    
>        This patch removes the support.  Since this routine only worked on
>        64-bit platforms but was only built on 32-bit platforms, it's
>        essentially just nonsense anyway.
>    
>        Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Acked-by: Nicolas Pitre <nico@fluxnic.net>

Related: there is still some opportunity for performance gain on rv32 by 
providing an assembly version of __arch_xprod_64() if someone is up to 
it.


Nicolas

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv