2018-01-17 06:51:28

by Antony Pavlov

[permalink] [raw]
Subject: [PATCH] MIPS: use generic GCC library routines from lib/

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 <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
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

menu "Machine selection"

diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
index 78c2affeabf8..195ab4cb0840 100644
--- a/arch/mips/lib/Makefile
+++ b/arch/mips/lib/Makefile
@@ -16,4 +16,4 @@ 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 += bswapsi.o bswapdi.o
diff --git a/arch/mips/lib/ashldi3.c b/arch/mips/lib/ashldi3.c
deleted file mode 100644
index 24cd6903e797..000000000000
--- a/arch/mips/lib/ashldi3.c
+++ /dev/null
@@ -1,30 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <linux/export.h>
-
-#include "libgcc.h"
-
-long long notrace __ashldi3(long long u, word_type b)
-{
- DWunion uu, w;
- word_type bm;
-
- if (b == 0)
- return u;
-
- uu.ll = u;
- bm = 32 - b;
-
- if (bm <= 0) {
- w.s.low = 0;
- w.s.high = (unsigned int) uu.s.low << -bm;
- } else {
- const unsigned int carries = (unsigned int) uu.s.low >> bm;
-
- w.s.low = (unsigned int) uu.s.low << b;
- w.s.high = ((unsigned int) uu.s.high << b) | carries;
- }
-
- return w.ll;
-}
-
-EXPORT_SYMBOL(__ashldi3);
diff --git a/arch/mips/lib/ashrdi3.c b/arch/mips/lib/ashrdi3.c
deleted file mode 100644
index 23f5295af51e..000000000000
--- a/arch/mips/lib/ashrdi3.c
+++ /dev/null
@@ -1,32 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <linux/export.h>
-
-#include "libgcc.h"
-
-long long notrace __ashrdi3(long long u, word_type b)
-{
- DWunion uu, w;
- word_type bm;
-
- if (b == 0)
- return u;
-
- uu.ll = u;
- bm = 32 - b;
-
- if (bm <= 0) {
- /* w.s.high = 1..1 or 0..0 */
- w.s.high =
- uu.s.high >> 31;
- w.s.low = uu.s.high >> -bm;
- } else {
- const unsigned int carries = (unsigned int) uu.s.high << bm;
-
- w.s.high = uu.s.high >> b;
- w.s.low = ((unsigned int) uu.s.low >> b) | carries;
- }
-
- return w.ll;
-}
-
-EXPORT_SYMBOL(__ashrdi3);
diff --git a/arch/mips/lib/cmpdi2.c b/arch/mips/lib/cmpdi2.c
deleted file mode 100644
index 93cfc785927d..000000000000
--- a/arch/mips/lib/cmpdi2.c
+++ /dev/null
@@ -1,28 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <linux/export.h>
-
-#include "libgcc.h"
-
-word_type notrace __cmpdi2(long long a, long long b)
-{
- const DWunion au = {
- .ll = a
- };
- const DWunion bu = {
- .ll = b
- };
-
- if (au.s.high < bu.s.high)
- return 0;
- else if (au.s.high > bu.s.high)
- return 2;
-
- if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
- return 0;
- else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
- return 2;
-
- return 1;
-}
-
-EXPORT_SYMBOL(__cmpdi2);
diff --git a/arch/mips/lib/lshrdi3.c b/arch/mips/lib/lshrdi3.c
deleted file mode 100644
index 914b971aca3b..000000000000
--- a/arch/mips/lib/lshrdi3.c
+++ /dev/null
@@ -1,30 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <linux/export.h>
-
-#include "libgcc.h"
-
-long long notrace __lshrdi3(long long u, word_type b)
-{
- DWunion uu, w;
- word_type bm;
-
- if (b == 0)
- return u;
-
- uu.ll = u;
- bm = 32 - b;
-
- if (bm <= 0) {
- w.s.high = 0;
- w.s.low = (unsigned int) uu.s.high >> -bm;
- } else {
- const unsigned int carries = (unsigned int) uu.s.high << bm;
-
- w.s.high = (unsigned int) uu.s.high >> b;
- w.s.low = ((unsigned int) uu.s.low >> b) | carries;
- }
-
- return w.ll;
-}
-
-EXPORT_SYMBOL(__lshrdi3);
diff --git a/arch/mips/lib/ucmpdi2.c b/arch/mips/lib/ucmpdi2.c
deleted file mode 100644
index c31c78ca4175..000000000000
--- 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)
-{
- const DWunion au = {.ll = a};
- const DWunion bu = {.ll = b};
-
- if ((unsigned int) au.s.high < (unsigned int) bu.s.high)
- return 0;
- else if ((unsigned int) au.s.high > (unsigned int) bu.s.high)
- return 2;
- if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
- return 0;
- else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
- return 2;
- return 1;
-}
-
-EXPORT_SYMBOL(__ucmpdi2);
--
2.15.1


2018-01-17 09:04:16

by Matt Redfearn

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

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 <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> 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

>
> menu "Machine selection"
>
> diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
> index 78c2affeabf8..195ab4cb0840 100644
> --- a/arch/mips/lib/Makefile
> +++ b/arch/mips/lib/Makefile
> @@ -16,4 +16,4 @@ 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 += bswapsi.o bswapdi.o
> diff --git a/arch/mips/lib/ashldi3.c b/arch/mips/lib/ashldi3.c
> deleted file mode 100644
> index 24cd6903e797..000000000000
> --- a/arch/mips/lib/ashldi3.c
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/export.h>
> -
> -#include "libgcc.h"
> -
> -long long notrace __ashldi3(long long u, word_type b)
> -{
> - DWunion uu, w;
> - word_type bm;
> -
> - if (b == 0)
> - return u;
> -
> - uu.ll = u;
> - bm = 32 - b;
> -
> - if (bm <= 0) {
> - w.s.low = 0;
> - w.s.high = (unsigned int) uu.s.low << -bm;
> - } else {
> - const unsigned int carries = (unsigned int) uu.s.low >> bm;
> -
> - w.s.low = (unsigned int) uu.s.low << b;
> - w.s.high = ((unsigned int) uu.s.high << b) | carries;
> - }
> -
> - return w.ll;
> -}
> -
> -EXPORT_SYMBOL(__ashldi3);
> diff --git a/arch/mips/lib/ashrdi3.c b/arch/mips/lib/ashrdi3.c
> deleted file mode 100644
> index 23f5295af51e..000000000000
> --- a/arch/mips/lib/ashrdi3.c
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/export.h>
> -
> -#include "libgcc.h"
> -
> -long long notrace __ashrdi3(long long u, word_type b)
> -{
> - DWunion uu, w;
> - word_type bm;
> -
> - if (b == 0)
> - return u;
> -
> - uu.ll = u;
> - bm = 32 - b;
> -
> - if (bm <= 0) {
> - /* w.s.high = 1..1 or 0..0 */
> - w.s.high =
> - uu.s.high >> 31;
> - w.s.low = uu.s.high >> -bm;
> - } else {
> - const unsigned int carries = (unsigned int) uu.s.high << bm;
> -
> - w.s.high = uu.s.high >> b;
> - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
> - }
> -
> - return w.ll;
> -}
> -
> -EXPORT_SYMBOL(__ashrdi3);
> diff --git a/arch/mips/lib/cmpdi2.c b/arch/mips/lib/cmpdi2.c
> deleted file mode 100644
> index 93cfc785927d..000000000000
> --- a/arch/mips/lib/cmpdi2.c
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/export.h>
> -
> -#include "libgcc.h"
> -
> -word_type notrace __cmpdi2(long long a, long long b)
> -{
> - const DWunion au = {
> - .ll = a
> - };
> - const DWunion bu = {
> - .ll = b
> - };
> -
> - if (au.s.high < bu.s.high)
> - return 0;
> - else if (au.s.high > bu.s.high)
> - return 2;
> -
> - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
> - return 0;
> - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
> - return 2;
> -
> - return 1;
> -}
> -
> -EXPORT_SYMBOL(__cmpdi2);
> diff --git a/arch/mips/lib/lshrdi3.c b/arch/mips/lib/lshrdi3.c
> deleted file mode 100644
> index 914b971aca3b..000000000000
> --- a/arch/mips/lib/lshrdi3.c
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/export.h>
> -
> -#include "libgcc.h"
> -
> -long long notrace __lshrdi3(long long u, word_type b)
> -{
> - DWunion uu, w;
> - word_type bm;
> -
> - if (b == 0)
> - return u;
> -
> - uu.ll = u;
> - bm = 32 - b;
> -
> - if (bm <= 0) {
> - w.s.high = 0;
> - w.s.low = (unsigned int) uu.s.high >> -bm;
> - } else {
> - const unsigned int carries = (unsigned int) uu.s.high << bm;
> -
> - w.s.high = (unsigned int) uu.s.high >> b;
> - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
> - }
> -
> - return w.ll;
> -}
> -
> -EXPORT_SYMBOL(__lshrdi3);
> diff --git a/arch/mips/lib/ucmpdi2.c b/arch/mips/lib/ucmpdi2.c
> deleted file mode 100644
> index c31c78ca4175..000000000000
> --- 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.

Thanks,
Matt

> -{
> - const DWunion au = {.ll = a};
> - const DWunion bu = {.ll = b};
> -
> - if ((unsigned int) au.s.high < (unsigned int) bu.s.high)
> - return 0;
> - else if ((unsigned int) au.s.high > (unsigned int) bu.s.high)
> - return 2;
> - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
> - return 0;
> - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
> - return 2;
> - return 1;
> -}
> -
> -EXPORT_SYMBOL(__ucmpdi2);
> --
> 2.15.1
>
>

2018-01-17 13:20:05

by Antony Pavlov

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

On Wed, 17 Jan 2018 09:03:48 +0000
Matt Redfearn <[email protected]> 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 <[email protected]>
> > Cc: Palmer Dabbelt <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > 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?
It looks like that nobody (even RISC-V code)
uses GENERIC_CMPDI2 and GENERIC_UCMPDI2. Why?
Do you use them in your local branches?

--
Best regards,
? Antony Pavlov

2018-01-18 01:35:42

by Palmer Dabbelt

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

Ah, thanks for reminding me -- I'd originally posted a patch set that converted
every other port to use these routines, but I ended up dropping all those.
Here's my original MIPS attempt

https://marc.info/?l=linux-mips&m=149677651707103&w=2

Given that, I think you can also drop arch/mips/lib/libgcc.h -- if it's used
from anywhere else, it should be possible to use include/linux/libgcc.h
instead.

Assuming it still seems sane do to that I can go give the rest of the patch set
another shot. I'm a bit new to this, but I think I should do something like

Reviewed-by: Palmer Dabbelt <[email protected]>

Thanks!

On Tue, 16 Jan 2018 22:51:21 PST (-0800), [email protected] 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 <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> 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
>
> menu "Machine selection"
>
> diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
> index 78c2affeabf8..195ab4cb0840 100644
> --- a/arch/mips/lib/Makefile
> +++ b/arch/mips/lib/Makefile
> @@ -16,4 +16,4 @@ 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 += bswapsi.o bswapdi.o
> diff --git a/arch/mips/lib/ashldi3.c b/arch/mips/lib/ashldi3.c
> deleted file mode 100644
> index 24cd6903e797..000000000000
> --- a/arch/mips/lib/ashldi3.c
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/export.h>
> -
> -#include "libgcc.h"
> -
> -long long notrace __ashldi3(long long u, word_type b)
> -{
> - DWunion uu, w;
> - word_type bm;
> -
> - if (b == 0)
> - return u;
> -
> - uu.ll = u;
> - bm = 32 - b;
> -
> - if (bm <= 0) {
> - w.s.low = 0;
> - w.s.high = (unsigned int) uu.s.low << -bm;
> - } else {
> - const unsigned int carries = (unsigned int) uu.s.low >> bm;
> -
> - w.s.low = (unsigned int) uu.s.low << b;
> - w.s.high = ((unsigned int) uu.s.high << b) | carries;
> - }
> -
> - return w.ll;
> -}
> -
> -EXPORT_SYMBOL(__ashldi3);
> diff --git a/arch/mips/lib/ashrdi3.c b/arch/mips/lib/ashrdi3.c
> deleted file mode 100644
> index 23f5295af51e..000000000000
> --- a/arch/mips/lib/ashrdi3.c
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/export.h>
> -
> -#include "libgcc.h"
> -
> -long long notrace __ashrdi3(long long u, word_type b)
> -{
> - DWunion uu, w;
> - word_type bm;
> -
> - if (b == 0)
> - return u;
> -
> - uu.ll = u;
> - bm = 32 - b;
> -
> - if (bm <= 0) {
> - /* w.s.high = 1..1 or 0..0 */
> - w.s.high =
> - uu.s.high >> 31;
> - w.s.low = uu.s.high >> -bm;
> - } else {
> - const unsigned int carries = (unsigned int) uu.s.high << bm;
> -
> - w.s.high = uu.s.high >> b;
> - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
> - }
> -
> - return w.ll;
> -}
> -
> -EXPORT_SYMBOL(__ashrdi3);
> diff --git a/arch/mips/lib/cmpdi2.c b/arch/mips/lib/cmpdi2.c
> deleted file mode 100644
> index 93cfc785927d..000000000000
> --- a/arch/mips/lib/cmpdi2.c
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/export.h>
> -
> -#include "libgcc.h"
> -
> -word_type notrace __cmpdi2(long long a, long long b)
> -{
> - const DWunion au = {
> - .ll = a
> - };
> - const DWunion bu = {
> - .ll = b
> - };
> -
> - if (au.s.high < bu.s.high)
> - return 0;
> - else if (au.s.high > bu.s.high)
> - return 2;
> -
> - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
> - return 0;
> - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
> - return 2;
> -
> - return 1;
> -}
> -
> -EXPORT_SYMBOL(__cmpdi2);
> diff --git a/arch/mips/lib/lshrdi3.c b/arch/mips/lib/lshrdi3.c
> deleted file mode 100644
> index 914b971aca3b..000000000000
> --- a/arch/mips/lib/lshrdi3.c
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/export.h>
> -
> -#include "libgcc.h"
> -
> -long long notrace __lshrdi3(long long u, word_type b)
> -{
> - DWunion uu, w;
> - word_type bm;
> -
> - if (b == 0)
> - return u;
> -
> - uu.ll = u;
> - bm = 32 - b;
> -
> - if (bm <= 0) {
> - w.s.high = 0;
> - w.s.low = (unsigned int) uu.s.high >> -bm;
> - } else {
> - const unsigned int carries = (unsigned int) uu.s.high << bm;
> -
> - w.s.high = (unsigned int) uu.s.high >> b;
> - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
> - }
> -
> - return w.ll;
> -}
> -
> -EXPORT_SYMBOL(__lshrdi3);
> diff --git a/arch/mips/lib/ucmpdi2.c b/arch/mips/lib/ucmpdi2.c
> deleted file mode 100644
> index c31c78ca4175..000000000000
> --- 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)
> -{
> - const DWunion au = {.ll = a};
> - const DWunion bu = {.ll = b};
> -
> - if ((unsigned int) au.s.high < (unsigned int) bu.s.high)
> - return 0;
> - else if ((unsigned int) au.s.high > (unsigned int) bu.s.high)
> - return 2;
> - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
> - return 0;
> - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
> - return 2;
> - return 1;
> -}
> -
> -EXPORT_SYMBOL(__ucmpdi2);

2018-01-18 16:29:43

by Palmer Dabbelt

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

On Wed, 17 Jan 2018 01:03:48 PST (-0800), [email protected] 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 <[email protected]>
>> Cc: Palmer Dabbelt <[email protected]>
>> Cc: Ralf Baechle <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> 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
>
>>
>> menu "Machine selection"
>>
>> diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
>> index 78c2affeabf8..195ab4cb0840 100644
>> --- a/arch/mips/lib/Makefile
>> +++ b/arch/mips/lib/Makefile
>> @@ -16,4 +16,4 @@ 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 += bswapsi.o bswapdi.o
>> diff --git a/arch/mips/lib/ashldi3.c b/arch/mips/lib/ashldi3.c
>> deleted file mode 100644
>> index 24cd6903e797..000000000000
>> --- a/arch/mips/lib/ashldi3.c
>> +++ /dev/null
>> @@ -1,30 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0
>> -#include <linux/export.h>
>> -
>> -#include "libgcc.h"
>> -
>> -long long notrace __ashldi3(long long u, word_type b)
>> -{
>> - DWunion uu, w;
>> - word_type bm;
>> -
>> - if (b == 0)
>> - return u;
>> -
>> - uu.ll = u;
>> - bm = 32 - b;
>> -
>> - if (bm <= 0) {
>> - w.s.low = 0;
>> - w.s.high = (unsigned int) uu.s.low << -bm;
>> - } else {
>> - const unsigned int carries = (unsigned int) uu.s.low >> bm;
>> -
>> - w.s.low = (unsigned int) uu.s.low << b;
>> - w.s.high = ((unsigned int) uu.s.high << b) | carries;
>> - }
>> -
>> - return w.ll;
>> -}
>> -
>> -EXPORT_SYMBOL(__ashldi3);
>> diff --git a/arch/mips/lib/ashrdi3.c b/arch/mips/lib/ashrdi3.c
>> deleted file mode 100644
>> index 23f5295af51e..000000000000
>> --- a/arch/mips/lib/ashrdi3.c
>> +++ /dev/null
>> @@ -1,32 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0
>> -#include <linux/export.h>
>> -
>> -#include "libgcc.h"
>> -
>> -long long notrace __ashrdi3(long long u, word_type b)
>> -{
>> - DWunion uu, w;
>> - word_type bm;
>> -
>> - if (b == 0)
>> - return u;
>> -
>> - uu.ll = u;
>> - bm = 32 - b;
>> -
>> - if (bm <= 0) {
>> - /* w.s.high = 1..1 or 0..0 */
>> - w.s.high =
>> - uu.s.high >> 31;
>> - w.s.low = uu.s.high >> -bm;
>> - } else {
>> - const unsigned int carries = (unsigned int) uu.s.high << bm;
>> -
>> - w.s.high = uu.s.high >> b;
>> - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
>> - }
>> -
>> - return w.ll;
>> -}
>> -
>> -EXPORT_SYMBOL(__ashrdi3);
>> diff --git a/arch/mips/lib/cmpdi2.c b/arch/mips/lib/cmpdi2.c
>> deleted file mode 100644
>> index 93cfc785927d..000000000000
>> --- a/arch/mips/lib/cmpdi2.c
>> +++ /dev/null
>> @@ -1,28 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0
>> -#include <linux/export.h>
>> -
>> -#include "libgcc.h"
>> -
>> -word_type notrace __cmpdi2(long long a, long long b)
>> -{
>> - const DWunion au = {
>> - .ll = a
>> - };
>> - const DWunion bu = {
>> - .ll = b
>> - };
>> -
>> - if (au.s.high < bu.s.high)
>> - return 0;
>> - else if (au.s.high > bu.s.high)
>> - return 2;
>> -
>> - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
>> - return 0;
>> - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
>> - return 2;
>> -
>> - return 1;
>> -}
>> -
>> -EXPORT_SYMBOL(__cmpdi2);
>> diff --git a/arch/mips/lib/lshrdi3.c b/arch/mips/lib/lshrdi3.c
>> deleted file mode 100644
>> index 914b971aca3b..000000000000
>> --- a/arch/mips/lib/lshrdi3.c
>> +++ /dev/null
>> @@ -1,30 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0
>> -#include <linux/export.h>
>> -
>> -#include "libgcc.h"
>> -
>> -long long notrace __lshrdi3(long long u, word_type b)
>> -{
>> - DWunion uu, w;
>> - word_type bm;
>> -
>> - if (b == 0)
>> - return u;
>> -
>> - uu.ll = u;
>> - bm = 32 - b;
>> -
>> - if (bm <= 0) {
>> - w.s.high = 0;
>> - w.s.low = (unsigned int) uu.s.high >> -bm;
>> - } else {
>> - const unsigned int carries = (unsigned int) uu.s.high << bm;
>> -
>> - w.s.high = (unsigned int) uu.s.high >> b;
>> - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
>> - }
>> -
>> - return w.ll;
>> -}
>> -
>> -EXPORT_SYMBOL(__lshrdi3);
>> diff --git a/arch/mips/lib/ucmpdi2.c b/arch/mips/lib/ucmpdi2.c
>> deleted file mode 100644
>> index c31c78ca4175..000000000000
>> --- 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.

IIRC we marked the others as notrace for a similar reason, I must have just
missed this one somehow. I can submit a patch.

Thanks!

>
> Thanks,
> Matt
>
>> -{
>> - const DWunion au = {.ll = a};
>> - const DWunion bu = {.ll = b};
>> -
>> - if ((unsigned int) au.s.high < (unsigned int) bu.s.high)
>> - return 0;
>> - else if ((unsigned int) au.s.high > (unsigned int) bu.s.high)
>> - return 2;
>> - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
>> - return 0;
>> - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
>> - return 2;
>> - return 1;
>> -}
>> -
>> -EXPORT_SYMBOL(__ucmpdi2);
>> --
>> 2.15.1
>>
>>

2018-01-18 16:29:46

by Palmer Dabbelt

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

On Wed, 17 Jan 2018 05:34:18 PST (-0800), [email protected] wrote:
> On Wed, 17 Jan 2018 09:03:48 +0000
> Matt Redfearn <[email protected]> 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 <[email protected]>
>> > Cc: Palmer Dabbelt <[email protected]>
>> > Cc: Ralf Baechle <[email protected]>
>> > Cc: [email protected]
>> > Cc: [email protected]
>> > ---
>> > 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 <[email protected]>
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 <[email protected]>
CC: Antony Pavlov <[email protected]>
Signed-off-by: Palmer Dabbelt <[email protected]>

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.

2018-01-18 19:51:31

by Antony Pavlov

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

On Wed, 17 Jan 2018 17:34:59 -0800 (PST)
Palmer Dabbelt <[email protected]> wrote:

> Ah, thanks for reminding me -- I'd originally posted a patch set that converted
> every other port to use these routines, but I ended up dropping all those.
> Here's my original MIPS attempt
>
> https://marc.info/?l=linux-mips&m=149677651707103&w=2

Now I see that I have dubbed your June 2017 patch. Sorry!

Alas in June 2017 there was no chance to push your patch to linux-mips repo.
Linus merged your patches in November 2017. Ralf merged your RISC-V changes
into linux-mips repo AFAIR just several days ago.

> Given that, I think you can also drop arch/mips/lib/libgcc.h -- if it's used
> from anywhere else, it should be possible to use include/linux/libgcc.h
> instead.

I agree with you.

>
> Assuming it still seems sane do to that I can go give the rest of the patch set
> another shot. I'm a bit new to this, but I think I should do something like
>
> Reviewed-by: Palmer Dabbelt <[email protected]>

I suppose that it's better to resurrect you patch and abandon my patch.

Your original patch series (https://www.linux-mips.org/archives/linux-mips/2017-06/msg00148.html)
has a disadvantage: it tries to change several architectures at once.

I propose to create new short separate patch series for MIPS architecture.

This patch series should contain two patches:

[PATCH 1/2] Add notrace to lib/ucmpdi2.c
[PATCH 2/2] MIPS: Use generic libgcc intrinsics


> Thanks!
>
> On Tue, 16 Jan 2018 22:51:21 PST (-0800), [email protected] 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 <[email protected]>
> > Cc: Palmer Dabbelt <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > 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
> >
> > menu "Machine selection"
> >
> > diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
> > index 78c2affeabf8..195ab4cb0840 100644
> > --- a/arch/mips/lib/Makefile
> > +++ b/arch/mips/lib/Makefile
> > @@ -16,4 +16,4 @@ 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 += bswapsi.o bswapdi.o
> > diff --git a/arch/mips/lib/ashldi3.c b/arch/mips/lib/ashldi3.c
> > deleted file mode 100644
> > index 24cd6903e797..000000000000
> > --- a/arch/mips/lib/ashldi3.c
> > +++ /dev/null
> > @@ -1,30 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -#include <linux/export.h>
> > -
> > -#include "libgcc.h"
> > -
> > -long long notrace __ashldi3(long long u, word_type b)
> > -{
> > - DWunion uu, w;
> > - word_type bm;
> > -
> > - if (b == 0)
> > - return u;
> > -
> > - uu.ll = u;
> > - bm = 32 - b;
> > -
> > - if (bm <= 0) {
> > - w.s.low = 0;
> > - w.s.high = (unsigned int) uu.s.low << -bm;
> > - } else {
> > - const unsigned int carries = (unsigned int) uu.s.low >> bm;
> > -
> > - w.s.low = (unsigned int) uu.s.low << b;
> > - w.s.high = ((unsigned int) uu.s.high << b) | carries;
> > - }
> > -
> > - return w.ll;
> > -}
> > -
> > -EXPORT_SYMBOL(__ashldi3);
> > diff --git a/arch/mips/lib/ashrdi3.c b/arch/mips/lib/ashrdi3.c
> > deleted file mode 100644
> > index 23f5295af51e..000000000000
> > --- a/arch/mips/lib/ashrdi3.c
> > +++ /dev/null
> > @@ -1,32 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -#include <linux/export.h>
> > -
> > -#include "libgcc.h"
> > -
> > -long long notrace __ashrdi3(long long u, word_type b)
> > -{
> > - DWunion uu, w;
> > - word_type bm;
> > -
> > - if (b == 0)
> > - return u;
> > -
> > - uu.ll = u;
> > - bm = 32 - b;
> > -
> > - if (bm <= 0) {
> > - /* w.s.high = 1..1 or 0..0 */
> > - w.s.high =
> > - uu.s.high >> 31;
> > - w.s.low = uu.s.high >> -bm;
> > - } else {
> > - const unsigned int carries = (unsigned int) uu.s.high << bm;
> > -
> > - w.s.high = uu.s.high >> b;
> > - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
> > - }
> > -
> > - return w.ll;
> > -}
> > -
> > -EXPORT_SYMBOL(__ashrdi3);
> > diff --git a/arch/mips/lib/cmpdi2.c b/arch/mips/lib/cmpdi2.c
> > deleted file mode 100644
> > index 93cfc785927d..000000000000
> > --- a/arch/mips/lib/cmpdi2.c
> > +++ /dev/null
> > @@ -1,28 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -#include <linux/export.h>
> > -
> > -#include "libgcc.h"
> > -
> > -word_type notrace __cmpdi2(long long a, long long b)
> > -{
> > - const DWunion au = {
> > - .ll = a
> > - };
> > - const DWunion bu = {
> > - .ll = b
> > - };
> > -
> > - if (au.s.high < bu.s.high)
> > - return 0;
> > - else if (au.s.high > bu.s.high)
> > - return 2;
> > -
> > - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
> > - return 0;
> > - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
> > - return 2;
> > -
> > - return 1;
> > -}
> > -
> > -EXPORT_SYMBOL(__cmpdi2);
> > diff --git a/arch/mips/lib/lshrdi3.c b/arch/mips/lib/lshrdi3.c
> > deleted file mode 100644
> > index 914b971aca3b..000000000000
> > --- a/arch/mips/lib/lshrdi3.c
> > +++ /dev/null
> > @@ -1,30 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -#include <linux/export.h>
> > -
> > -#include "libgcc.h"
> > -
> > -long long notrace __lshrdi3(long long u, word_type b)
> > -{
> > - DWunion uu, w;
> > - word_type bm;
> > -
> > - if (b == 0)
> > - return u;
> > -
> > - uu.ll = u;
> > - bm = 32 - b;
> > -
> > - if (bm <= 0) {
> > - w.s.high = 0;
> > - w.s.low = (unsigned int) uu.s.high >> -bm;
> > - } else {
> > - const unsigned int carries = (unsigned int) uu.s.high << bm;
> > -
> > - w.s.high = (unsigned int) uu.s.high >> b;
> > - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
> > - }
> > -
> > - return w.ll;
> > -}
> > -
> > -EXPORT_SYMBOL(__lshrdi3);
> > diff --git a/arch/mips/lib/ucmpdi2.c b/arch/mips/lib/ucmpdi2.c
> > deleted file mode 100644
> > index c31c78ca4175..000000000000
> > --- 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)
> > -{
> > - const DWunion au = {.ll = a};
> > - const DWunion bu = {.ll = b};
> > -
> > - if ((unsigned int) au.s.high < (unsigned int) bu.s.high)
> > - return 0;
> > - else if ((unsigned int) au.s.high > (unsigned int) bu.s.high)
> > - return 2;
> > - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
> > - return 0;
> > - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
> > - return 2;
> > - return 1;
> > -}
> > -
> > -EXPORT_SYMBOL(__ucmpdi2);


--
Best regards,
? Antony Pavlov

2018-01-18 19:55:41

by James Hogan

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

On 18 January 2018 20:05:05 GMT+00:00, Antony Pavlov <[email protected]> wrote:
>On Wed, 17 Jan 2018 17:34:59 -0800 (PST)
>Palmer Dabbelt <[email protected]> wrote:
>
>> Ah, thanks for reminding me -- I'd originally posted a patch set that
>converted
>> every other port to use these routines, but I ended up dropping all
>those.
>> Here's my original MIPS attempt
>>
>> https://marc.info/?l=linux-mips&m=149677651707103&w=2
>
>Now I see that I have dubbed your June 2017 patch. Sorry!
>
>Alas in June 2017 there was no chance to push your patch to linux-mips
>repo.
>Linus merged your patches in November 2017. Ralf merged your RISC-V
>changes
>into linux-mips repo AFAIR just several days ago.
>
>> Given that, I think you can also drop arch/mips/lib/libgcc.h -- if
>it's used
>> from anywhere else, it should be possible to use
>include/linux/libgcc.h
>> instead.
>
>I agree with you.

actually theres a patch in mips-next which implements __multi3 for mips64r6, which uses that file, and in fact extends it for 128bit types.

cheers
James

>
>>
>> Assuming it still seems sane do to that I can go give the rest of the
>patch set
>> another shot. I'm a bit new to this, but I think I should do
>something like
>>
>> Reviewed-by: Palmer Dabbelt <[email protected]>
>
>I suppose that it's better to resurrect you patch and abandon my patch.
>
>Your original patch series
>(https://www.linux-mips.org/archives/linux-mips/2017-06/msg00148.html)
>has a disadvantage: it tries to change several architectures at once.
>
>I propose to create new short separate patch series for MIPS
>architecture.
>
>This patch series should contain two patches:
>
> [PATCH 1/2] Add notrace to lib/ucmpdi2.c
> [PATCH 2/2] MIPS: Use generic libgcc intrinsics
>
>
>> Thanks!
>>
>> On Tue, 16 Jan 2018 22:51:21 PST (-0800), [email protected]
>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 <[email protected]>
>> > Cc: Palmer Dabbelt <[email protected]>
>> > Cc: Ralf Baechle <[email protected]>
>> > Cc: [email protected]
>> > Cc: [email protected]
>> > ---
>> > 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
>> >
>> > menu "Machine selection"
>> >
>> > diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
>> > index 78c2affeabf8..195ab4cb0840 100644
>> > --- a/arch/mips/lib/Makefile
>> > +++ b/arch/mips/lib/Makefile
>> > @@ -16,4 +16,4 @@ 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 += bswapsi.o bswapdi.o
>> > diff --git a/arch/mips/lib/ashldi3.c b/arch/mips/lib/ashldi3.c
>> > deleted file mode 100644
>> > index 24cd6903e797..000000000000
>> > --- a/arch/mips/lib/ashldi3.c
>> > +++ /dev/null
>> > @@ -1,30 +0,0 @@
>> > -// SPDX-License-Identifier: GPL-2.0
>> > -#include <linux/export.h>
>> > -
>> > -#include "libgcc.h"
>> > -
>> > -long long notrace __ashldi3(long long u, word_type b)
>> > -{
>> > - DWunion uu, w;
>> > - word_type bm;
>> > -
>> > - if (b == 0)
>> > - return u;
>> > -
>> > - uu.ll = u;
>> > - bm = 32 - b;
>> > -
>> > - if (bm <= 0) {
>> > - w.s.low = 0;
>> > - w.s.high = (unsigned int) uu.s.low << -bm;
>> > - } else {
>> > - const unsigned int carries = (unsigned int) uu.s.low >> bm;
>> > -
>> > - w.s.low = (unsigned int) uu.s.low << b;
>> > - w.s.high = ((unsigned int) uu.s.high << b) | carries;
>> > - }
>> > -
>> > - return w.ll;
>> > -}
>> > -
>> > -EXPORT_SYMBOL(__ashldi3);
>> > diff --git a/arch/mips/lib/ashrdi3.c b/arch/mips/lib/ashrdi3.c
>> > deleted file mode 100644
>> > index 23f5295af51e..000000000000
>> > --- a/arch/mips/lib/ashrdi3.c
>> > +++ /dev/null
>> > @@ -1,32 +0,0 @@
>> > -// SPDX-License-Identifier: GPL-2.0
>> > -#include <linux/export.h>
>> > -
>> > -#include "libgcc.h"
>> > -
>> > -long long notrace __ashrdi3(long long u, word_type b)
>> > -{
>> > - DWunion uu, w;
>> > - word_type bm;
>> > -
>> > - if (b == 0)
>> > - return u;
>> > -
>> > - uu.ll = u;
>> > - bm = 32 - b;
>> > -
>> > - if (bm <= 0) {
>> > - /* w.s.high = 1..1 or 0..0 */
>> > - w.s.high =
>> > - uu.s.high >> 31;
>> > - w.s.low = uu.s.high >> -bm;
>> > - } else {
>> > - const unsigned int carries = (unsigned int) uu.s.high << bm;
>> > -
>> > - w.s.high = uu.s.high >> b;
>> > - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
>> > - }
>> > -
>> > - return w.ll;
>> > -}
>> > -
>> > -EXPORT_SYMBOL(__ashrdi3);
>> > diff --git a/arch/mips/lib/cmpdi2.c b/arch/mips/lib/cmpdi2.c
>> > deleted file mode 100644
>> > index 93cfc785927d..000000000000
>> > --- a/arch/mips/lib/cmpdi2.c
>> > +++ /dev/null
>> > @@ -1,28 +0,0 @@
>> > -// SPDX-License-Identifier: GPL-2.0
>> > -#include <linux/export.h>
>> > -
>> > -#include "libgcc.h"
>> > -
>> > -word_type notrace __cmpdi2(long long a, long long b)
>> > -{
>> > - const DWunion au = {
>> > - .ll = a
>> > - };
>> > - const DWunion bu = {
>> > - .ll = b
>> > - };
>> > -
>> > - if (au.s.high < bu.s.high)
>> > - return 0;
>> > - else if (au.s.high > bu.s.high)
>> > - return 2;
>> > -
>> > - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
>> > - return 0;
>> > - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
>> > - return 2;
>> > -
>> > - return 1;
>> > -}
>> > -
>> > -EXPORT_SYMBOL(__cmpdi2);
>> > diff --git a/arch/mips/lib/lshrdi3.c b/arch/mips/lib/lshrdi3.c
>> > deleted file mode 100644
>> > index 914b971aca3b..000000000000
>> > --- a/arch/mips/lib/lshrdi3.c
>> > +++ /dev/null
>> > @@ -1,30 +0,0 @@
>> > -// SPDX-License-Identifier: GPL-2.0
>> > -#include <linux/export.h>
>> > -
>> > -#include "libgcc.h"
>> > -
>> > -long long notrace __lshrdi3(long long u, word_type b)
>> > -{
>> > - DWunion uu, w;
>> > - word_type bm;
>> > -
>> > - if (b == 0)
>> > - return u;
>> > -
>> > - uu.ll = u;
>> > - bm = 32 - b;
>> > -
>> > - if (bm <= 0) {
>> > - w.s.high = 0;
>> > - w.s.low = (unsigned int) uu.s.high >> -bm;
>> > - } else {
>> > - const unsigned int carries = (unsigned int) uu.s.high << bm;
>> > -
>> > - w.s.high = (unsigned int) uu.s.high >> b;
>> > - w.s.low = ((unsigned int) uu.s.low >> b) | carries;
>> > - }
>> > -
>> > - return w.ll;
>> > -}
>> > -
>> > -EXPORT_SYMBOL(__lshrdi3);
>> > diff --git a/arch/mips/lib/ucmpdi2.c b/arch/mips/lib/ucmpdi2.c
>> > deleted file mode 100644
>> > index c31c78ca4175..000000000000
>> > --- 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)
>> > -{
>> > - const DWunion au = {.ll = a};
>> > - const DWunion bu = {.ll = b};
>> > -
>> > - if ((unsigned int) au.s.high < (unsigned int) bu.s.high)
>> > - return 0;
>> > - else if ((unsigned int) au.s.high > (unsigned int) bu.s.high)
>> > - return 2;
>> > - if ((unsigned int) au.s.low < (unsigned int) bu.s.low)
>> > - return 0;
>> > - else if ((unsigned int) au.s.low > (unsigned int) bu.s.low)
>> > - return 2;
>> > - return 1;
>> > -}
>> > -
>> > -EXPORT_SYMBOL(__ucmpdi2);


--
Cheers
James Hogan

2018-01-18 20:27:09

by James Hogan

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

On Thu, Jan 18, 2018 at 07:54:49PM +0000, James Hogan wrote:
> On 18 January 2018 20:05:05 GMT+00:00, Antony Pavlov <[email protected]> wrote:
> >On Wed, 17 Jan 2018 17:34:59 -0800 (PST)
> >Palmer Dabbelt <[email protected]> wrote:
> >> Given that, I think you can also drop arch/mips/lib/libgcc.h -- if
> >it's used
> >> from anywhere else, it should be possible to use
> >include/linux/libgcc.h
> >> instead.
> >
> >I agree with you.
>
> actually theres a patch in mips-next which implements __multi3 for mips64r6, which uses that file, and in fact extends it for 128bit types.

More specifically, commit ebabcf17bcd7 ("MIPS: Implement __multi3 for
GCC7 MIPS64r6 builds"), which will find its way into v4.15. See also
https://patchwork.linux-mips.org/patch/17890/

Cheers
James


Attachments:
(No filename) (809.00 B)
signature.asc (849.00 B)
Digital signature
Download all attachments

2018-01-18 20:31:17

by James Hogan

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

On Wed, Jan 17, 2018 at 04:34:18PM +0300, Antony Pavlov wrote:
> On Wed, 17 Jan 2018 09:03:48 +0000
> Matt Redfearn <[email protected]> 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 <[email protected]>
> > > Cc: Palmer Dabbelt <[email protected]>
> > > Cc: Ralf Baechle <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > ---
> > > 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?

FWIW, with both matt's comments addressed:
Reviewed-by: James Hogan <[email protected]>

Cheers
James

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


Attachments:
(No filename) (3.38 kB)
signature.asc (849.00 B)
Digital signature
Download all attachments