| Message ID | 74dbe68dc8e2ffb6180092f73723fe21ab692c7a.1679566500.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] sh: Use generic GCC library routines | expand |
Hi Geert,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.3-rc4 next-20230327]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Geert-Uytterhoeven/sh-Use-generic-GCC-library-routines/20230323-181932
patch link: https://lore.kernel.org/r/74dbe68dc8e2ffb6180092f73723fe21ab692c7a.1679566500.git.geert%2Brenesas%40glider.be
patch subject: [PATCH v3] sh: Use generic GCC library routines
config: sh-allnoconfig (https://download.01.org/0day-ci/archive/20230327/202303272214.RxzpA6bP-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f81d82f320398d37e233429781ed14069e3eaf53
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Geert-Uytterhoeven/sh-Use-generic-GCC-library-routines/20230323-181932
git checkout f81d82f320398d37e233429781ed14069e3eaf53
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303272214.RxzpA6bP-lkp@intel.com/
Note: functions only called from assembly code should be annotated with the asmlinkage attribute
All warnings (new ones prefixed by >>):
In file included from arch/sh/boot/compressed/ashldi3.c:2:
>> arch/sh/boot/compressed/../../../../lib/ashldi3.c:9:19: warning: no previous prototype for '__ashldi3' [-Wmissing-prototypes]
9 | long long notrace __ashldi3(long long u, word_type b)
| ^~~~~~~~~
vim +/__ashldi3 +9 arch/sh/boot/compressed/../../../../lib/ashldi3.c
b35cd9884fa5d8 Palmer Dabbelt 2017-05-23 8
b35cd9884fa5d8 Palmer Dabbelt 2017-05-23 @9 long long notrace __ashldi3(long long u, word_type b)
Hi Geert! On Thu, 2023-03-23 at 11:18 +0100, Geert Uytterhoeven wrote: > The C implementations of __ashldi3(), __ashrdi3__(), and __lshrdi3() in > arch/sh/lib/ are identical to the generic C implementations in lib/. > Reduce duplication by switching SH to the generic versions. > > Update the include path in arch/sh/boot/compressed accordingly. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > --- > v3: > - Add Acked-by, > > v2: > - Fix silly typo in subject. > > Tested on landisk and qemu/rts7751r2d. > > Note that it also works without the change to arch/sh/boot/compressed/, > as lib/ashldi3.c can be reached via both include/uapi/../../lib/ashldi3.c > and arch/sh/boot/compressed/../../../../lib/ashldi3.c. > > Palmer tried a similar thing before: > https://lore.kernel.org/linux-arch/20170523220546.16758-1-palmer@dabbelt.com/ > but initially it broke the SH build due to a missing change to > arch/sh/boot/compressed/, and the later update never got picked up. > In the mean time, arch/sh/boot/compressed/ was changed, so his patch no > longer applies. > > Similar for the other architectures, I guess? > --- > arch/sh/Kconfig | 3 +++ > arch/sh/boot/compressed/ashldi3.c | 4 ++-- > arch/sh/lib/Makefile | 4 +--- > arch/sh/lib/ashldi3.c | 30 ----------------------------- > arch/sh/lib/ashrdi3.c | 32 ------------------------------- > arch/sh/lib/lshrdi3.c | 30 ----------------------------- > 6 files changed, 6 insertions(+), 97 deletions(-) > delete mode 100644 arch/sh/lib/ashldi3.c > delete mode 100644 arch/sh/lib/ashrdi3.c > delete mode 100644 arch/sh/lib/lshrdi3.c > > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig > index ccb866750a884643..892903dc74990c36 100644 > --- a/arch/sh/Kconfig > +++ b/arch/sh/Kconfig > @@ -20,6 +20,9 @@ config SUPERH > select GENERIC_CMOS_UPDATE if SH_SH03 || SH_DREAMCAST > select GENERIC_IDLE_POLL_SETUP > select GENERIC_IRQ_SHOW > + select GENERIC_LIB_ASHLDI3 > + select GENERIC_LIB_ASHRDI3 > + select GENERIC_LIB_LSHRDI3 > select GENERIC_PCI_IOMAP if PCI > select GENERIC_SCHED_CLOCK > select GENERIC_SMP_IDLE_THREAD > diff --git a/arch/sh/boot/compressed/ashldi3.c b/arch/sh/boot/compressed/ashldi3.c > index 7cebd646df839b48..7c12121702309e8c 100644 > --- a/arch/sh/boot/compressed/ashldi3.c > +++ b/arch/sh/boot/compressed/ashldi3.c > @@ -1,2 +1,2 @@ > -// SPDX-License-Identifier: GPL-2.0-only > -#include "../../lib/ashldi3.c" > +// SPDX-License-Identifier: GPL-2.0-or-later > +#include "../../../../lib/ashldi3.c" > diff --git a/arch/sh/lib/Makefile b/arch/sh/lib/Makefile > index eb473d373ca43a4b..d20a0768b31fa2b6 100644 > --- a/arch/sh/lib/Makefile > +++ b/arch/sh/lib/Makefile > @@ -7,9 +7,7 @@ lib-y = delay.o memmove.o memchr.o \ > checksum.o strlen.o div64.o div64-generic.o > > # Extracted from libgcc > -obj-y += movmem.o ashldi3.o ashrdi3.o lshrdi3.o \ > - ashlsi3.o ashrsi3.o ashiftrt.o lshrsi3.o \ > - udiv_qrnnd.o > +obj-y += movmem.o ashlsi3.o ashrsi3.o ashiftrt.o lshrsi3.o udiv_qrnnd.o Why are the single-precision (denoted as "si") variants not being replaced? Don't we have generic versions for these? Adrian
> On Apr 21, 2023, at 20:03, John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote: > > Hi Geert! > > On Thu, 2023-03-23 at 11:18 +0100, Geert Uytterhoeven wrote: >> The C implementations of __ashldi3(), __ashrdi3__(), and __lshrdi3() in >> arch/sh/lib/ are identical to the generic C implementations in lib/. >> Reduce duplication by switching SH to the generic versions. ... > > Why are the single-precision (denoted as "si") variants not being replaced? > > Don't we have generic versions for these? The SH arch versions of si variants are optimized assembler, which is pretty important… these are things like bit shifts. Actually, it would be better to have the di variants be hand coded asm also… I’m not sure how much use the kernel makes of those, and I’ve not looked if there is a good source of optimized SH versions those with the right license. J. > > Adrian > > -- > .''`. John Paul Adrian Glaubitz > : :' : Debian Developer > `. `' Physicist > `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Hi Adrian, On Fri, Apr 21, 2023 at 1:03 PM John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote: > On Thu, 2023-03-23 at 11:18 +0100, Geert Uytterhoeven wrote: > > The C implementations of __ashldi3(), __ashrdi3__(), and __lshrdi3() in > > arch/sh/lib/ are identical to the generic C implementations in lib/. > > Reduce duplication by switching SH to the generic versions. > > > > Update the include path in arch/sh/boot/compressed accordingly. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > --- a/arch/sh/lib/Makefile > > +++ b/arch/sh/lib/Makefile > > @@ -7,9 +7,7 @@ lib-y = delay.o memmove.o memchr.o \ > > checksum.o strlen.o div64.o div64-generic.o > > > > # Extracted from libgcc > > -obj-y += movmem.o ashldi3.o ashrdi3.o lshrdi3.o \ > > - ashlsi3.o ashrsi3.o ashiftrt.o lshrsi3.o \ > > - udiv_qrnnd.o > > +obj-y += movmem.o ashlsi3.o ashrsi3.o ashiftrt.o lshrsi3.o udiv_qrnnd.o > > Why are the single-precision (denoted as "si") variants not being replaced? > > Don't we have generic versions for these? Because they are written in assembler, and thus different from generic versions, and because Linux does not include generic versions for these. Gr{oetje,eeting}s, Geert
On Thu, 2023-03-23 at 11:18 +0100, Geert Uytterhoeven wrote: > The C implementations of __ashldi3(), __ashrdi3__(), and __lshrdi3() in > arch/sh/lib/ are identical to the generic C implementations in lib/. > Reduce duplication by switching SH to the generic versions. > > Update the include path in arch/sh/boot/compressed accordingly. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > --- > v3: > - Add Acked-by, > > v2: > - Fix silly typo in subject. > > Tested on landisk and qemu/rts7751r2d. > > Note that it also works without the change to arch/sh/boot/compressed/, > as lib/ashldi3.c can be reached via both include/uapi/../../lib/ashldi3.c > and arch/sh/boot/compressed/../../../../lib/ashldi3.c. > > Palmer tried a similar thing before: > https://lore.kernel.org/linux-arch/20170523220546.16758-1-palmer@dabbelt.com/ > but initially it broke the SH build due to a missing change to > arch/sh/boot/compressed/, and the later update never got picked up. > In the mean time, arch/sh/boot/compressed/ was changed, so his patch no > longer applies. > > Similar for the other architectures, I guess? > --- > arch/sh/Kconfig | 3 +++ > arch/sh/boot/compressed/ashldi3.c | 4 ++-- > arch/sh/lib/Makefile | 4 +--- > arch/sh/lib/ashldi3.c | 30 ----------------------------- > arch/sh/lib/ashrdi3.c | 32 ------------------------------- > arch/sh/lib/lshrdi3.c | 30 ----------------------------- > 6 files changed, 6 insertions(+), 97 deletions(-) > delete mode 100644 arch/sh/lib/ashldi3.c > delete mode 100644 arch/sh/lib/ashrdi3.c > delete mode 100644 arch/sh/lib/lshrdi3.c > > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig > index ccb866750a884643..892903dc74990c36 100644 > --- a/arch/sh/Kconfig > +++ b/arch/sh/Kconfig > @@ -20,6 +20,9 @@ config SUPERH > select GENERIC_CMOS_UPDATE if SH_SH03 || SH_DREAMCAST > select GENERIC_IDLE_POLL_SETUP > select GENERIC_IRQ_SHOW > + select GENERIC_LIB_ASHLDI3 > + select GENERIC_LIB_ASHRDI3 > + select GENERIC_LIB_LSHRDI3 > select GENERIC_PCI_IOMAP if PCI > select GENERIC_SCHED_CLOCK > select GENERIC_SMP_IDLE_THREAD > diff --git a/arch/sh/boot/compressed/ashldi3.c b/arch/sh/boot/compressed/ashldi3.c > index 7cebd646df839b48..7c12121702309e8c 100644 > --- a/arch/sh/boot/compressed/ashldi3.c > +++ b/arch/sh/boot/compressed/ashldi3.c > @@ -1,2 +1,2 @@ > -// SPDX-License-Identifier: GPL-2.0-only > -#include "../../lib/ashldi3.c" > +// SPDX-License-Identifier: GPL-2.0-or-later > +#include "../../../../lib/ashldi3.c" > diff --git a/arch/sh/lib/Makefile b/arch/sh/lib/Makefile > index eb473d373ca43a4b..d20a0768b31fa2b6 100644 > --- a/arch/sh/lib/Makefile > +++ b/arch/sh/lib/Makefile > @@ -7,9 +7,7 @@ lib-y = delay.o memmove.o memchr.o \ > checksum.o strlen.o div64.o div64-generic.o > > # Extracted from libgcc > -obj-y += movmem.o ashldi3.o ashrdi3.o lshrdi3.o \ > - ashlsi3.o ashrsi3.o ashiftrt.o lshrsi3.o \ > - udiv_qrnnd.o > +obj-y += movmem.o ashlsi3.o ashrsi3.o ashiftrt.o lshrsi3.o udiv_qrnnd.o > > udivsi3-y := udivsi3_i4i-Os.o > > diff --git a/arch/sh/lib/ashldi3.c b/arch/sh/lib/ashldi3.c > deleted file mode 100644 > index e5afe0935847427f..0000000000000000 > --- a/arch/sh/lib/ashldi3.c > +++ /dev/null > @@ -1,30 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0 > -#include <linux/module.h> > - > -#include "libgcc.h" > - > -long long __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/sh/lib/ashrdi3.c b/arch/sh/lib/ashrdi3.c > deleted file mode 100644 > index ae263fbf25383b70..0000000000000000 > --- a/arch/sh/lib/ashrdi3.c > +++ /dev/null > @@ -1,32 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0 > -#include <linux/module.h> > - > -#include "libgcc.h" > - > -long long __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/sh/lib/lshrdi3.c b/arch/sh/lib/lshrdi3.c > deleted file mode 100644 > index 33eaa1edbc3c0656..0000000000000000 > --- a/arch/sh/lib/lshrdi3.c > +++ /dev/null > @@ -1,30 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0 > -#include <linux/module.h> > - > -#include "libgcc.h" > - > -long long __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); Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig index ccb866750a884643..892903dc74990c36 100644 --- a/arch/sh/Kconfig +++ b/arch/sh/Kconfig @@ -20,6 +20,9 @@ config SUPERH select GENERIC_CMOS_UPDATE if SH_SH03 || SH_DREAMCAST select GENERIC_IDLE_POLL_SETUP select GENERIC_IRQ_SHOW + select GENERIC_LIB_ASHLDI3 + select GENERIC_LIB_ASHRDI3 + select GENERIC_LIB_LSHRDI3 select GENERIC_PCI_IOMAP if PCI select GENERIC_SCHED_CLOCK select GENERIC_SMP_IDLE_THREAD diff --git a/arch/sh/boot/compressed/ashldi3.c b/arch/sh/boot/compressed/ashldi3.c index 7cebd646df839b48..7c12121702309e8c 100644 --- a/arch/sh/boot/compressed/ashldi3.c +++ b/arch/sh/boot/compressed/ashldi3.c @@ -1,2 +1,2 @@ -// SPDX-License-Identifier: GPL-2.0-only -#include "../../lib/ashldi3.c" +// SPDX-License-Identifier: GPL-2.0-or-later +#include "../../../../lib/ashldi3.c" diff --git a/arch/sh/lib/Makefile b/arch/sh/lib/Makefile index eb473d373ca43a4b..d20a0768b31fa2b6 100644 --- a/arch/sh/lib/Makefile +++ b/arch/sh/lib/Makefile @@ -7,9 +7,7 @@ lib-y = delay.o memmove.o memchr.o \ checksum.o strlen.o div64.o div64-generic.o # Extracted from libgcc -obj-y += movmem.o ashldi3.o ashrdi3.o lshrdi3.o \ - ashlsi3.o ashrsi3.o ashiftrt.o lshrsi3.o \ - udiv_qrnnd.o +obj-y += movmem.o ashlsi3.o ashrsi3.o ashiftrt.o lshrsi3.o udiv_qrnnd.o udivsi3-y := udivsi3_i4i-Os.o diff --git a/arch/sh/lib/ashldi3.c b/arch/sh/lib/ashldi3.c deleted file mode 100644 index e5afe0935847427f..0000000000000000 --- a/arch/sh/lib/ashldi3.c +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include <linux/module.h> - -#include "libgcc.h" - -long long __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/sh/lib/ashrdi3.c b/arch/sh/lib/ashrdi3.c deleted file mode 100644 index ae263fbf25383b70..0000000000000000 --- a/arch/sh/lib/ashrdi3.c +++ /dev/null @@ -1,32 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include <linux/module.h> - -#include "libgcc.h" - -long long __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/sh/lib/lshrdi3.c b/arch/sh/lib/lshrdi3.c deleted file mode 100644 index 33eaa1edbc3c0656..0000000000000000 --- a/arch/sh/lib/lshrdi3.c +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include <linux/module.h> - -#include "libgcc.h" - -long long __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);