Re: [PATCH] MIPS: use generic GCC library routines from lib/

[Palmer Dabbelt]

On Wed, 17 Jan 2018 05:34:18 PST (-0800), antonynpavlov@xxxxxxxxx wrote:
> On Wed, 17 Jan 2018 09:03:48 +0000
> Matt Redfearn <matt.redfearn@xxxxxxxx> wrote:
>
> > Hi,
> >
> > On Wed, Jan 17, 2018 at 09:51:21AM +0300, Antony Pavlov wrote:
> > > The commit b35cd9884fa5 ("lib: Add shared copies of
> > > some GCC library routines") makes it possible
> > > to share generic GCC library routines by several
> > > architectures.
> > > 
> > > This commit removes several generic GCC library
> > > routines from arch/mips/lib/ in favour of similar
> > > routines from lib/.
> > > 
> > > Signed-off-by: Antony Pavlov <antonynpavlov@xxxxxxxxx>
> > > Cc: Palmer Dabbelt <palmer@xxxxxxxxxx>
> > > Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> > > Cc: linux-mips@xxxxxxxxxxxxxx
> > > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > > ---
> > >  arch/mips/Kconfig       |  5 +++++
> > >  arch/mips/lib/Makefile  |  2 +-
> > >  arch/mips/lib/ashldi3.c | 30 ------------------------------
> > >  arch/mips/lib/ashrdi3.c | 32 --------------------------------
> > >  arch/mips/lib/cmpdi2.c  | 28 ----------------------------
> > >  arch/mips/lib/lshrdi3.c | 30 ------------------------------
> > >  arch/mips/lib/ucmpdi2.c | 22 ----------------------
> > >  7 files changed, 6 insertions(+), 143 deletions(-)
> > >  delete mode 100644 arch/mips/lib/ashldi3.c
> > >  delete mode 100644 arch/mips/lib/ashrdi3.c
> > >  delete mode 100644 arch/mips/lib/cmpdi2.c
> > >  delete mode 100644 arch/mips/lib/lshrdi3.c
> > >  delete mode 100644 arch/mips/lib/ucmpdi2.c
> > > 
> > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > > index 350a990fc719..9cd49ee848c6 100644
> > > --- a/arch/mips/Kconfig
> > > +++ b/arch/mips/Kconfig
> > > @@ -73,6 +73,11 @@ config MIPS
> > >  	select RTC_LIB if !MACH_LOONGSON64
> > >  	select SYSCTL_EXCEPTION_TRACE
> > >  	select VIRT_TO_BUS
> > > +	select GENERIC_ASHLDI3
> > > +	select GENERIC_ASHRDI3
> > > +	select GENERIC_LSHRDI3
> > > +	select GENERIC_CMPDI2
> > > +	select GENERIC_UCMPDI2
> >
> > Please preserve alphabetical order
>
> Ok, I'll fix order in v2 patch.
>
> > > --- a/arch/mips/lib/ucmpdi2.c
> > > +++ /dev/null
> > > @@ -1,22 +0,0 @@
> > > -// SPDX-License-Identifier: GPL-2.0
> > > -#include <linux/export.h>
> > > -
> > > -#include "libgcc.h"
> > > -
> > > -word_type notrace __ucmpdi2(unsigned long long a, unsigned long long b)
> >
> > The version of __ucmpdi2 in /lib/ is not marked notrace. We have seen
> > issues before with compiler intrinsics not being marked notrace - see
> > aedcfbe06558 ("MIPS: lib: Mark intrinsics notrace")
> >
> > Please ensure that the /lib/ version is equivalent before switching to
> > that version.
>
> Good shot! I have missed this 'notrace'.
>
> lib/ucmpdi2.c differ from other GCC library routines files from lib
> related to my patch (ashldi3.c, ashrdi3.c, cmpdi2.c, lshrdi3.c):
> only lib/ucmpdi2.c has no 'notrace' flag. In other details the files
> look equivalent. The files arch/mips/lib/libgcc.h and include/linux/libgcc.h
> have no fundamental differences.
>
> to Palmer:
> Can we add notrace to lib/ucmpdi2.c?

Works for me.  Do you want to add a patch to this set?  Since it's a dependency 
of this patch it seems to make a bit more sense to just keep here.  Mine looks like

   commit dba01764391cbd6e636595f7b957357b2cf2c14a
   Author: Palmer Dabbelt <palmer@xxxxxxxxxx>
   Date:   Thu Jan 18 07:47:35 2018 -0800
   
       Add notrace to lib/ucmpdi2.c
   
       As part of the MIPS conversion to use the generic GCC library routines,
       Matt Redfearn discovered that I'd missed a notrace on __ucmpdi2().  This
       patch rectifies the problem.
   
       CC: Matt Redfearn <matt.redfearn@xxxxxxxx>
       CC: Antony Pavlov <antonynpavlov@xxxxxxxxx>
       Signed-off-by: Palmer Dabbelt <palmer@xxxxxxxxxx>
   
   diff --git a/lib/ucmpdi2.c b/lib/ucmpdi2.c
   index 25ca2d4c1e19..597998169a96 100644
   --- a/lib/ucmpdi2.c
   +++ b/lib/ucmpdi2.c
   @@ -17,7 +17,7 @@
    #include <linux/module.h>
    #include <linux/libgcc.h>
   
   -word_type __ucmpdi2(unsigned long long a, unsigned long long b)
   +word_type notrace __ucmpdi2(unsigned long long a, unsigned long long b)
    {
    	const DWunion au = {.ll = a};
    	const DWunion bu = {.ll = b};

> It looks like that nobody (even RISC-V code)
> uses GENERIC_CMPDI2 and GENERIC_UCMPDI2. Why?
> Do you use them in your local branches?

I'd meant to convert every arch port over to using the generic routines, but it 
sort of just got lost in the shuffle.  I'll resurrect that patch set.