2022-08-30 19:37:22

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 1/2] Documentation: raise minimum supported version of binutils to 2.25.1

Binutils 2.23 was released in 2012. Almost 10 years old.

We already require GCC 5.1, which was released in 2015.

Bump the binutils version to 2.25.1, which was also released in 2015.

Suggested-by: Nick Desaulniers <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---

Documentation/process/changes.rst | 4 ++--
scripts/min-tool-version.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
index 19c286c23786..5fb6a60677ef 100644
--- a/Documentation/process/changes.rst
+++ b/Documentation/process/changes.rst
@@ -33,7 +33,7 @@ GNU C 5.1 gcc --version
Clang/LLVM (optional) 11.0.0 clang --version
GNU make 3.81 make --version
bash 4.2 bash --version
-binutils 2.23 ld -v
+binutils 2.25.1 ld -v
flex 2.5.35 flex --version
bison 2.0 bison --version
pahole 1.16 pahole --version
@@ -94,7 +94,7 @@ Bash 4.2 or newer is needed.
Binutils
--------

-Binutils 2.23 or newer is needed to build the kernel.
+Binutils 2.25.1 or newer is needed to build the kernel.

pkg-config
----------
diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
index 250925aab101..8b1edd1f7281 100755
--- a/scripts/min-tool-version.sh
+++ b/scripts/min-tool-version.sh
@@ -14,7 +14,7 @@ fi

case "$1" in
binutils)
- echo 2.23.0
+ echo 2.25.1
;;
gcc)
echo 5.1.0
--
2.34.1


2022-08-30 19:38:09

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 2/2] powerpc: remove old code for binutils < 2.25

The minimum supported version of binutils has been raised to 2.25.1.
Drop the old code.

PPC is the last user of ld-ifversion. With all the callers removed,
the macro definition in scripts/Makefile.compiler can go away.

Signed-off-by: Masahiro Yamada <[email protected]>
---

arch/powerpc/Makefile | 21 ---------------------
arch/powerpc/lib/Makefile | 8 --------
scripts/Makefile.compiler | 4 ----
3 files changed, 33 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 02742facf895..fb607758eeca 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -46,13 +46,7 @@ UTS_MACHINE := $(subst $(space),,$(machine-y))
ifdef CONFIG_PPC32
KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
else
-ifeq ($(call ld-ifversion, -ge, 22500, y),y)
-# Have the linker provide sfpr if possible.
-# There is a corresponding test in arch/powerpc/lib/Makefile
KBUILD_LDFLAGS_MODULE += --save-restore-funcs
-else
-KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
-endif
endif

ifdef CONFIG_CPU_LITTLE_ENDIAN
@@ -395,8 +389,6 @@ vdso_prepare: prepare0
$(build)=arch/powerpc/kernel/vdso include/generated/vdso64-offsets.h)
endif

-archprepare: checkbin
-
archheaders:
$(Q)$(MAKE) $(build)=arch/powerpc/kernel/syscalls all

@@ -411,16 +403,3 @@ else
$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
endif
endif
-
-PHONY += checkbin
-# Check toolchain versions:
-# - gcc-4.6 is the minimum kernel-wide version so nothing required.
-checkbin:
- @if test "x${CONFIG_LD_IS_LLD}" != "xy" -a \
- "x$(call ld-ifversion, -le, 22400, y)" = "xy" ; then \
- echo -n '*** binutils 2.24 miscompiles weak symbols ' ; \
- echo 'in some circumstances.' ; \
- echo '*** binutils 2.23 do not define the TOC symbol ' ; \
- echo -n '*** Please use a different binutils version.' ; \
- false ; \
- fi
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 8560c912186d..5eb3971ccb9c 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -38,14 +38,6 @@ obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o

obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o

-# See corresponding test in arch/powerpc/Makefile
-# 64-bit linker creates .sfpr on demand for final link (vmlinux),
-# so it is only needed for modules, and only for older linkers which
-# do not support --save-restore-funcs
-ifeq ($(call ld-ifversion, -lt, 22500, y),y)
-extra-$(CONFIG_PPC64) += crtsavres.o
-endif
-
obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
memcpy_power7.o restart_table.o

diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
index 94d0d40cddb3..63e7d79dd877 100644
--- a/scripts/Makefile.compiler
+++ b/scripts/Makefile.compiler
@@ -68,7 +68,3 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e
# ld-option
# Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
-
-# ld-ifversion
-# Usage: $(call ld-ifversion, -ge, 22252, y)
-ld-ifversion = $(shell [ $(CONFIG_LD_VERSION)0 $(1) $(2)0 ] && echo $(3) || echo $(4))
--
2.34.1

2022-08-30 22:20:53

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: remove old code for binutils < 2.25

On Tue, Aug 30, 2022 at 12:10 PM Masahiro Yamada <[email protected]> wrote:
>
> The minimum supported version of binutils has been raised to 2.25.1.
> Drop the old code.
>
> PPC is the last user of ld-ifversion. With all the callers removed,
> the macro definition in scripts/Makefile.compiler can go away.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> arch/powerpc/Makefile | 21 ---------------------
> arch/powerpc/lib/Makefile | 8 --------
> scripts/Makefile.compiler | 4 ----
> 3 files changed, 33 deletions(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 02742facf895..fb607758eeca 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -46,13 +46,7 @@ UTS_MACHINE := $(subst $(space),,$(machine-y))
> ifdef CONFIG_PPC32
> KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> else
> -ifeq ($(call ld-ifversion, -ge, 22500, y),y)
> -# Have the linker provide sfpr if possible.

^ Perhaps this comment is still relevant and should not yet be
discarded? Or updated, dropping " if possible".

Either way:
Reviewed-by: Nick Desaulniers <[email protected]>

> -# There is a corresponding test in arch/powerpc/lib/Makefile
> KBUILD_LDFLAGS_MODULE += --save-restore-funcs
> -else
> -KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> -endif
> endif
>
> ifdef CONFIG_CPU_LITTLE_ENDIAN
> @@ -395,8 +389,6 @@ vdso_prepare: prepare0
> $(build)=arch/powerpc/kernel/vdso include/generated/vdso64-offsets.h)
> endif
>
> -archprepare: checkbin
> -
> archheaders:
> $(Q)$(MAKE) $(build)=arch/powerpc/kernel/syscalls all
>
> @@ -411,16 +403,3 @@ else
> $(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
> endif
> endif
> -
> -PHONY += checkbin
> -# Check toolchain versions:
> -# - gcc-4.6 is the minimum kernel-wide version so nothing required.
> -checkbin:
> - @if test "x${CONFIG_LD_IS_LLD}" != "xy" -a \
> - "x$(call ld-ifversion, -le, 22400, y)" = "xy" ; then \
> - echo -n '*** binutils 2.24 miscompiles weak symbols ' ; \
> - echo 'in some circumstances.' ; \
> - echo '*** binutils 2.23 do not define the TOC symbol ' ; \
> - echo -n '*** Please use a different binutils version.' ; \
> - false ; \
> - fi
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 8560c912186d..5eb3971ccb9c 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -38,14 +38,6 @@ obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o
>
> obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
>
> -# See corresponding test in arch/powerpc/Makefile
> -# 64-bit linker creates .sfpr on demand for final link (vmlinux),
> -# so it is only needed for modules, and only for older linkers which
> -# do not support --save-restore-funcs
> -ifeq ($(call ld-ifversion, -lt, 22500, y),y)
> -extra-$(CONFIG_PPC64) += crtsavres.o
> -endif
> -
> obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
> memcpy_power7.o restart_table.o
>
> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> index 94d0d40cddb3..63e7d79dd877 100644
> --- a/scripts/Makefile.compiler
> +++ b/scripts/Makefile.compiler
> @@ -68,7 +68,3 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e
> # ld-option
> # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
> ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
> -
> -# ld-ifversion
> -# Usage: $(call ld-ifversion, -ge, 22252, y)
> -ld-ifversion = $(shell [ $(CONFIG_LD_VERSION)0 $(1) $(2)0 ] && echo $(3) || echo $(4))
> --
> 2.34.1
>


--
Thanks,
~Nick Desaulniers

2022-08-30 22:22:07

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: remove old code for binutils < 2.25

On Tue, Aug 30, 2022 at 2:44 PM Nathan Chancellor <[email protected]> wrote:
>
> On Tue, Aug 30, 2022 at 02:13:20PM -0700, Nick Desaulniers wrote:
> > On Tue, Aug 30, 2022 at 12:10 PM Masahiro Yamada <[email protected]> wrote:
> > >
> > > The minimum supported version of binutils has been raised to 2.25.1.
> > > Drop the old code.
> > >
> > > PPC is the last user of ld-ifversion. With all the callers removed,
> > > the macro definition in scripts/Makefile.compiler can go away.
> > >
> > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > ---
> > >
> > > arch/powerpc/Makefile | 21 ---------------------
> > > arch/powerpc/lib/Makefile | 8 --------
> > > scripts/Makefile.compiler | 4 ----
> > > 3 files changed, 33 deletions(-)
> > >
> > > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > > index 02742facf895..fb607758eeca 100644
> > > --- a/arch/powerpc/Makefile
> > > +++ b/arch/powerpc/Makefile
> > > @@ -46,13 +46,7 @@ UTS_MACHINE := $(subst $(space),,$(machine-y))
> > > ifdef CONFIG_PPC32
> > > KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > > else
> > > -ifeq ($(call ld-ifversion, -ge, 22500, y),y)
> > > -# Have the linker provide sfpr if possible.
> >
> > ^ Perhaps this comment is still relevant and should not yet be
> > discarded? Or updated, dropping " if possible".
> >
> > Either way:
> > Reviewed-by: Nick Desaulniers <[email protected]>
>
> I think we still want this block for ld.lld. Prior to this change,
> ld.lld would fail the ld-ifversion check and use crtsavres.o. Now, it
> will try to use '--save-restore-funcs', which it doesn't have support
> for, breaking the build for at least powernv_defconfig:
>
> ld.lld: error: unknown argument '--save-restore-funcs'

Good catch, this should probably be rewritten to use ld-option then.

I wonder what LLD's default behavior is, and if we need a feature request filed?

>
> > > -# There is a corresponding test in arch/powerpc/lib/Makefile
> > > KBUILD_LDFLAGS_MODULE += --save-restore-funcs
> > > -else
> > > -KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > > -endif
> > > endif
> > >
> > > ifdef CONFIG_CPU_LITTLE_ENDIAN
> > > @@ -395,8 +389,6 @@ vdso_prepare: prepare0
> > > $(build)=arch/powerpc/kernel/vdso include/generated/vdso64-offsets.h)
> > > endif
> > >
> > > -archprepare: checkbin
> > > -
> > > archheaders:
> > > $(Q)$(MAKE) $(build)=arch/powerpc/kernel/syscalls all
> > >
> > > @@ -411,16 +403,3 @@ else
> > > $(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
> > > endif
> > > endif
> > > -
> > > -PHONY += checkbin
> > > -# Check toolchain versions:
> > > -# - gcc-4.6 is the minimum kernel-wide version so nothing required.
> > > -checkbin:
> > > - @if test "x${CONFIG_LD_IS_LLD}" != "xy" -a \
> > > - "x$(call ld-ifversion, -le, 22400, y)" = "xy" ; then \
> > > - echo -n '*** binutils 2.24 miscompiles weak symbols ' ; \
> > > - echo 'in some circumstances.' ; \
> > > - echo '*** binutils 2.23 do not define the TOC symbol ' ; \
> > > - echo -n '*** Please use a different binutils version.' ; \
> > > - false ; \
> > > - fi
> > > diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> > > index 8560c912186d..5eb3971ccb9c 100644
> > > --- a/arch/powerpc/lib/Makefile
> > > +++ b/arch/powerpc/lib/Makefile
> > > @@ -38,14 +38,6 @@ obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o
> > >
> > > obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
> > >
> > > -# See corresponding test in arch/powerpc/Makefile
> > > -# 64-bit linker creates .sfpr on demand for final link (vmlinux),
> > > -# so it is only needed for modules, and only for older linkers which
> > > -# do not support --save-restore-funcs
> > > -ifeq ($(call ld-ifversion, -lt, 22500, y),y)
> > > -extra-$(CONFIG_PPC64) += crtsavres.o
> > > -endif
> > > -
> > > obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
> > > memcpy_power7.o restart_table.o
> > >
> > > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> > > index 94d0d40cddb3..63e7d79dd877 100644
> > > --- a/scripts/Makefile.compiler
> > > +++ b/scripts/Makefile.compiler
> > > @@ -68,7 +68,3 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e
> > > # ld-option
> > > # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
> > > ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
> > > -
> > > -# ld-ifversion
> > > -# Usage: $(call ld-ifversion, -ge, 22252, y)
> > > -ld-ifversion = $(shell [ $(CONFIG_LD_VERSION)0 $(1) $(2)0 ] && echo $(3) || echo $(4))
> > > --
> > > 2.34.1
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers



--
Thanks,
~Nick Desaulniers

2022-08-30 22:46:16

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 1/2] Documentation: raise minimum supported version of binutils to 2.25.1

On Tue, Aug 30, 2022 at 12:10 PM Masahiro Yamada <[email protected]> wrote:
>
> Binutils 2.23 was released in 2012. Almost 10 years old.
>
> We already require GCC 5.1, which was released in 2015.
>
> Bump the binutils version to 2.25.1, which was also released in 2015.
>
> Suggested-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Documentation/process/changes.rst | 4 ++--

This document has also been translated into Italian. Let's keep them in sync?
Documentation/translations/it_IT/process/changes.rst

Either way.
Acked-by: Nick Desaulniers <[email protected]>

grepping for binutils shows lots of potentially stale references to
even older versions of binutils. There may be more cleanups lurking
than just the ppc ld-ifversion check.


> scripts/min-tool-version.sh | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
> index 19c286c23786..5fb6a60677ef 100644
> --- a/Documentation/process/changes.rst
> +++ b/Documentation/process/changes.rst
> @@ -33,7 +33,7 @@ GNU C 5.1 gcc --version
> Clang/LLVM (optional) 11.0.0 clang --version
> GNU make 3.81 make --version
> bash 4.2 bash --version
> -binutils 2.23 ld -v
> +binutils 2.25.1 ld -v
> flex 2.5.35 flex --version
> bison 2.0 bison --version
> pahole 1.16 pahole --version
> @@ -94,7 +94,7 @@ Bash 4.2 or newer is needed.
> Binutils
> --------
>
> -Binutils 2.23 or newer is needed to build the kernel.
> +Binutils 2.25.1 or newer is needed to build the kernel.
>
> pkg-config
> ----------
> diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
> index 250925aab101..8b1edd1f7281 100755
> --- a/scripts/min-tool-version.sh
> +++ b/scripts/min-tool-version.sh
> @@ -14,7 +14,7 @@ fi
>
> case "$1" in
> binutils)
> - echo 2.23.0
> + echo 2.25.1
> ;;
> gcc)
> echo 5.1.0
> --
> 2.34.1
>


--
Thanks,
~Nick Desaulniers

2022-08-30 22:47:05

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: remove old code for binutils < 2.25

On Tue, Aug 30, 2022 at 02:13:20PM -0700, Nick Desaulniers wrote:
> On Tue, Aug 30, 2022 at 12:10 PM Masahiro Yamada <[email protected]> wrote:
> >
> > The minimum supported version of binutils has been raised to 2.25.1.
> > Drop the old code.
> >
> > PPC is the last user of ld-ifversion. With all the callers removed,
> > the macro definition in scripts/Makefile.compiler can go away.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > arch/powerpc/Makefile | 21 ---------------------
> > arch/powerpc/lib/Makefile | 8 --------
> > scripts/Makefile.compiler | 4 ----
> > 3 files changed, 33 deletions(-)
> >
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index 02742facf895..fb607758eeca 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -46,13 +46,7 @@ UTS_MACHINE := $(subst $(space),,$(machine-y))
> > ifdef CONFIG_PPC32
> > KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > else
> > -ifeq ($(call ld-ifversion, -ge, 22500, y),y)
> > -# Have the linker provide sfpr if possible.
>
> ^ Perhaps this comment is still relevant and should not yet be
> discarded? Or updated, dropping " if possible".
>
> Either way:
> Reviewed-by: Nick Desaulniers <[email protected]>

I think we still want this block for ld.lld. Prior to this change,
ld.lld would fail the ld-ifversion check and use crtsavres.o. Now, it
will try to use '--save-restore-funcs', which it doesn't have support
for, breaking the build for at least powernv_defconfig:

ld.lld: error: unknown argument '--save-restore-funcs'

> > -# There is a corresponding test in arch/powerpc/lib/Makefile
> > KBUILD_LDFLAGS_MODULE += --save-restore-funcs
> > -else
> > -KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > -endif
> > endif
> >
> > ifdef CONFIG_CPU_LITTLE_ENDIAN
> > @@ -395,8 +389,6 @@ vdso_prepare: prepare0
> > $(build)=arch/powerpc/kernel/vdso include/generated/vdso64-offsets.h)
> > endif
> >
> > -archprepare: checkbin
> > -
> > archheaders:
> > $(Q)$(MAKE) $(build)=arch/powerpc/kernel/syscalls all
> >
> > @@ -411,16 +403,3 @@ else
> > $(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
> > endif
> > endif
> > -
> > -PHONY += checkbin
> > -# Check toolchain versions:
> > -# - gcc-4.6 is the minimum kernel-wide version so nothing required.
> > -checkbin:
> > - @if test "x${CONFIG_LD_IS_LLD}" != "xy" -a \
> > - "x$(call ld-ifversion, -le, 22400, y)" = "xy" ; then \
> > - echo -n '*** binutils 2.24 miscompiles weak symbols ' ; \
> > - echo 'in some circumstances.' ; \
> > - echo '*** binutils 2.23 do not define the TOC symbol ' ; \
> > - echo -n '*** Please use a different binutils version.' ; \
> > - false ; \
> > - fi
> > diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> > index 8560c912186d..5eb3971ccb9c 100644
> > --- a/arch/powerpc/lib/Makefile
> > +++ b/arch/powerpc/lib/Makefile
> > @@ -38,14 +38,6 @@ obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o
> >
> > obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
> >
> > -# See corresponding test in arch/powerpc/Makefile
> > -# 64-bit linker creates .sfpr on demand for final link (vmlinux),
> > -# so it is only needed for modules, and only for older linkers which
> > -# do not support --save-restore-funcs
> > -ifeq ($(call ld-ifversion, -lt, 22500, y),y)
> > -extra-$(CONFIG_PPC64) += crtsavres.o
> > -endif
> > -
> > obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
> > memcpy_power7.o restart_table.o
> >
> > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> > index 94d0d40cddb3..63e7d79dd877 100644
> > --- a/scripts/Makefile.compiler
> > +++ b/scripts/Makefile.compiler
> > @@ -68,7 +68,3 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e
> > # ld-option
> > # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
> > ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
> > -
> > -# ld-ifversion
> > -# Usage: $(call ld-ifversion, -ge, 22252, y)
> > -ld-ifversion = $(shell [ $(CONFIG_LD_VERSION)0 $(1) $(2)0 ] && echo $(3) || echo $(4))
> > --
> > 2.34.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers

2022-08-31 01:35:28

by Fangrui Song

[permalink] [raw]
Subject: Re: [PATCH 2/2] powerpc: remove old code for binutils < 2.25

On 2022-08-30, Nick Desaulniers wrote:
>On Tue, Aug 30, 2022 at 2:44 PM Nathan Chancellor <[email protected]> wrote:
>>
>> On Tue, Aug 30, 2022 at 02:13:20PM -0700, Nick Desaulniers wrote:
>> > On Tue, Aug 30, 2022 at 12:10 PM Masahiro Yamada <[email protected]> wrote:
>> > >
>> > > The minimum supported version of binutils has been raised to 2.25.1.
>> > > Drop the old code.
>> > >
>> > > PPC is the last user of ld-ifversion. With all the callers removed,
>> > > the macro definition in scripts/Makefile.compiler can go away.
>> > >
>> > > Signed-off-by: Masahiro Yamada <[email protected]>
>> > > ---
>> > >
>> > > arch/powerpc/Makefile | 21 ---------------------
>> > > arch/powerpc/lib/Makefile | 8 --------
>> > > scripts/Makefile.compiler | 4 ----
>> > > 3 files changed, 33 deletions(-)
>> > >
>> > > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> > > index 02742facf895..fb607758eeca 100644
>> > > --- a/arch/powerpc/Makefile
>> > > +++ b/arch/powerpc/Makefile
>> > > @@ -46,13 +46,7 @@ UTS_MACHINE := $(subst $(space),,$(machine-y))
>> > > ifdef CONFIG_PPC32
>> > > KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
>> > > else
>> > > -ifeq ($(call ld-ifversion, -ge, 22500, y),y)
>> > > -# Have the linker provide sfpr if possible.
>> >
>> > ^ Perhaps this comment is still relevant and should not yet be
>> > discarded? Or updated, dropping " if possible".
>> >
>> > Either way:
>> > Reviewed-by: Nick Desaulniers <[email protected]>
>>
>> I think we still want this block for ld.lld. Prior to this change,
>> ld.lld would fail the ld-ifversion check and use crtsavres.o. Now, it
>> will try to use '--save-restore-funcs', which it doesn't have support
>> for, breaking the build for at least powernv_defconfig:
>>
>> ld.lld: error: unknown argument '--save-restore-funcs'
>
>Good catch, this should probably be rewritten to use ld-option then.
>
>I wonder what LLD's default behavior is, and if we need a feature request filed?

I have implemented the save/restore functions[1] in ld.lld
(https://reviews.llvm.org/D79977) but do not add --(no-)?save-restore-funcs.
ld.lld's behavior matches GNU ld default: for non-relocatable links,
save/restore functions are synthesized.

ld.bfd --save-restore-funcs changes a relocatable link to define save/restore
functions. Is this behavior sufficiently useful? I don't think the obscure
linker option --save-restore-funcs is used anywhere else (checked with Debian
Code Search).


[1]: In the 64-bit ELF V2 API Specification: Power Architecture, 2.3.3.1. GPR
Save and Restore Functions defines some special functions which may be
referenced by GCC produced assembly (LLVM does not reference them).


>>
>> > > -# There is a corresponding test in arch/powerpc/lib/Makefile
>> > > KBUILD_LDFLAGS_MODULE += --save-restore-funcs
>> > > -else
>> > > -KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
>> > > -endif
>> > > endif
>> > >
>> > > ifdef CONFIG_CPU_LITTLE_ENDIAN
>> > > @@ -395,8 +389,6 @@ vdso_prepare: prepare0
>> > > $(build)=arch/powerpc/kernel/vdso include/generated/vdso64-offsets.h)
>> > > endif
>> > >
>> > > -archprepare: checkbin
>> > > -
>> > > archheaders:
>> > > $(Q)$(MAKE) $(build)=arch/powerpc/kernel/syscalls all
>> > >
>> > > @@ -411,16 +403,3 @@ else
>> > > $(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
>> > > endif
>> > > endif
>> > > -
>> > > -PHONY += checkbin
>> > > -# Check toolchain versions:
>> > > -# - gcc-4.6 is the minimum kernel-wide version so nothing required.
>> > > -checkbin:
>> > > - @if test "x${CONFIG_LD_IS_LLD}" != "xy" -a \
>> > > - "x$(call ld-ifversion, -le, 22400, y)" = "xy" ; then \
>> > > - echo -n '*** binutils 2.24 miscompiles weak symbols ' ; \
>> > > - echo 'in some circumstances.' ; \
>> > > - echo '*** binutils 2.23 do not define the TOC symbol ' ; \
>> > > - echo -n '*** Please use a different binutils version.' ; \
>> > > - false ; \
>> > > - fi
>> > > diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
>> > > index 8560c912186d..5eb3971ccb9c 100644
>> > > --- a/arch/powerpc/lib/Makefile
>> > > +++ b/arch/powerpc/lib/Makefile
>> > > @@ -38,14 +38,6 @@ obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o
>> > >
>> > > obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
>> > >
>> > > -# See corresponding test in arch/powerpc/Makefile
>> > > -# 64-bit linker creates .sfpr on demand for final link (vmlinux),
>> > > -# so it is only needed for modules, and only for older linkers which
>> > > -# do not support --save-restore-funcs
>> > > -ifeq ($(call ld-ifversion, -lt, 22500, y),y)
>> > > -extra-$(CONFIG_PPC64) += crtsavres.o
>> > > -endif
>> > > -
>> > > obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
>> > > memcpy_power7.o restart_table.o
>> > >
>> > > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
>> > > index 94d0d40cddb3..63e7d79dd877 100644
>> > > --- a/scripts/Makefile.compiler
>> > > +++ b/scripts/Makefile.compiler
>> > > @@ -68,7 +68,3 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e
>> > > # ld-option
>> > > # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
>> > > ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
>> > > -
>> > > -# ld-ifversion
>> > > -# Usage: $(call ld-ifversion, -ge, 22252, y)
>> > > -ld-ifversion = $(shell [ $(CONFIG_LD_VERSION)0 $(1) $(2)0 ] && echo $(3) || echo $(4))
>> > > --
>> > > 2.34.1
>> > >
>> >
>> >
>> > --
>> > Thanks,
>> > ~Nick Desaulniers
>
>
>
>--
>Thanks,
>~Nick Desaulniers