LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/3] LLVM/Clang fixes for pseries_defconfig
@ 2019-09-11 18:20 Nathan Chancellor
  2019-09-11 18:20 ` [PATCH v3 1/3] powerpc: Don't add -mabi= flags when building with Clang Nathan Chancellor
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Nathan Chancellor @ 2019-09-11 18:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: clang-built-linux, Nick Desaulniers, linuxppc-dev, linux-kernel

Hi all,

This series includes a set of fixes for LLVM/Clang when building
pseries_defconfig. These have been floating around as standalone patches
so I decided to gather them up as a series so it was easier to
review/apply them. The versioning is a bit wonky because of this reason,
I have included the previous versions of the patches below as well as
added an explanation on each patch. Please let me know if there are any
comments or concerns.

Previous postings:

https://lore.kernel.org/lkml/20190818191321.58185-1-natechancellor@gmail.com/
https://lore.kernel.org/lkml/20190820232921.102673-1-natechancellor@gmail.com/
https://lore.kernel.org/lkml/20190812023214.107817-1-natechancellor@gmail.com/

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 1/3] powerpc: Don't add -mabi= flags when building with Clang
  2019-09-11 18:20 [PATCH v3 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
@ 2019-09-11 18:20 ` Nathan Chancellor
  2019-09-11 18:20 ` [PATCH v3 2/3] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2019-09-11 18:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux,
	Nathan Chancellor, linuxppc-dev, Daniel Axtens

When building pseries_defconfig, building vdso32 errors out:

  error: unknown target ABI 'elfv1'

This happens because -m32 in clang changes the target to 32-bit,
which does not allow the ABI to be changed, as the setABI virtual
function is not overridden:

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/include/clang/Basic/TargetInfo.h#L1073-L1078

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Basic/Targets/PPC.h#L327-L365

Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
powerpc64le toolchain") added these flags to fix building big endian
kernels with a little endian GCC.

Clang doesn't need -mabi because the target triple controls the default
value. -mlittle-endian and -mbig-endian manipulate the triple into
either powerpc64-* or powerpc64le-*, which properly sets the default
ABI:

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Driver/Driver.cpp#L450-L463

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/llvm/lib/Support/Triple.cpp#L1432-L1516

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Basic/Targets/PPC.h#L377-L383

Adding a debug print out in the PPC64TargetInfo constructor after line
383 above shows this:

$ echo | ./clang -E --target=powerpc64-linux -mbig-endian -o /dev/null -
Default ABI: elfv1

$ echo | ./clang -E --target=powerpc64-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2

$ echo | ./clang -E --target=powerpc64le-linux -mbig-endian -o /dev/null -
Default ABI: elfv1

$ echo | ./clang -E --target=powerpc64le-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2

Don't specify -mabi when building with clang to avoid the build error
with -m32 and not change any code generation.

-mcall-aixdesc is not an implemented flag in clang so it can be
safely excluded as well, see commit 238abecde8ad ("powerpc: Don't
use gcc specific options on clang").

pseries_defconfig successfully builds after this patch and
powernv_defconfig and ppc44x_defconfig don't regress.

Link: https://github.com/ClangBuiltLinux/linux/issues/240
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

v1 -> v2:

* Improve commit message

v2 -> v3:

* Rebase and merge into a single series.

 arch/powerpc/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 46ed198a3aa3..150925a2e06e 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -93,11 +93,13 @@ MULTIPLEWORD	:= -mmultiple
 endif
 
 ifdef CONFIG_PPC64
+ifndef CONFIG_CC_IS_CLANG
 cflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mabi=elfv1)
 cflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mcall-aixdesc)
 aflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mabi=elfv1)
 aflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mabi=elfv2
 endif
+endif
 
 ifndef CONFIG_CC_IS_CLANG
   cflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mno-strict-align
@@ -143,6 +145,7 @@ endif
 endif
 
 CFLAGS-$(CONFIG_PPC64)	:= $(call cc-option,-mtraceback=no)
+ifndef CONFIG_CC_IS_CLANG
 ifdef CONFIG_CPU_LITTLE_ENDIAN
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv2,$(call cc-option,-mcall-aixdesc))
 AFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv2)
@@ -151,6 +154,7 @@ CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv1)
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mcall-aixdesc)
 AFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv1)
 endif
+endif
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mcmodel=medium,$(call cc-option,-mminimal-toc))
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mno-pointers-to-nested-functions)
 
-- 
2.23.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 2/3] powerpc: Avoid clang warnings around setjmp and longjmp
  2019-09-11 18:20 [PATCH v3 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
  2019-09-11 18:20 ` [PATCH v3 1/3] powerpc: Don't add -mabi= flags when building with Clang Nathan Chancellor
@ 2019-09-11 18:20 ` Nathan Chancellor
  2019-09-11 20:55   ` Nick Desaulniers
  2019-09-11 18:20 ` [PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp Nathan Chancellor
  2019-10-14  2:50 ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
  3 siblings, 1 reply; 21+ messages in thread
From: Nathan Chancellor @ 2019-09-11 18:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Nick Desaulniers, linux-kernel, stable, clang-built-linux,
	Nathan Chancellor, linuxppc-dev

Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
setjmp is used") disabled -Wbuiltin-requires-header because of a warning
about the setjmp and longjmp declarations.

r367387 in clang added another diagnostic around this, complaining that
there is no jmp_buf declaration.

In file included from ../arch/powerpc/xmon/xmon.c:47:
../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
built-in function 'setjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header <setjmp.h>.
[-Werror,-Wincomplete-setjmp-declaration]
extern long setjmp(long *);
            ^
../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
built-in function 'longjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header <setjmp.h>.
[-Werror,-Wincomplete-setjmp-declaration]
extern void longjmp(long *, long);
            ^
2 errors generated.

We are not using the standard library's longjmp/setjmp implementations
for obvious reasons; make this clear to clang by using -ffreestanding
on these files.

Cc: stable@vger.kernel.org # 4.14+
Link: https://github.com/ClangBuiltLinux/linux/issues/625
Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

v1 -> v3:

* Use -ffreestanding instead of outright disabling the warning because
  it is legitimate.

I skipped v2 because the first patch in the series already had a v2.

 arch/powerpc/kernel/Makefile | 4 ++--
 arch/powerpc/xmon/Makefile   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index c9cc4b689e60..19f19c8c874b 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -5,8 +5,8 @@
 
 CFLAGS_ptrace.o		+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
 
-# Disable clang warning for using setjmp without setjmp.h header
-CFLAGS_crash.o		+= $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+CFLAGS_crash.o		+= -ffreestanding
 
 ifdef CONFIG_PPC64
 CFLAGS_prom_init.o	+= $(NO_MINIMAL_TOC)
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index f142570ad860..c3842dbeb1b7 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for xmon
 
-# Disable clang warning for using setjmp without setjmp.h header
-subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+subdir-ccflags-y := -ffreestanding
 
 GCOV_PROFILE := n
 KCOV_INSTRUMENT := n
-- 
2.23.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-09-11 18:20 [PATCH v3 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
  2019-09-11 18:20 ` [PATCH v3 1/3] powerpc: Don't add -mabi= flags when building with Clang Nathan Chancellor
  2019-09-11 18:20 ` [PATCH v3 2/3] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
@ 2019-09-11 18:20 ` Nathan Chancellor
  2019-09-11 21:01   ` Nick Desaulniers
  2019-10-14  2:50 ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
  3 siblings, 1 reply; 21+ messages in thread
From: Nathan Chancellor @ 2019-09-11 18:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: clang-built-linux, Nathan Chancellor, Nick Desaulniers,
	linuxppc-dev, linux-kernel

r370454 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: https://github.com/ClangBuiltLinux/linux/issues/647
Link: https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

New patch in the series so no previous version.

 arch/powerpc/kernel/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 19f19c8c874b..aa78b3f6271e 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -21,7 +21,7 @@ CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 
-CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
+CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector) -ffreestanding
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
-- 
2.23.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 2/3] powerpc: Avoid clang warnings around setjmp and longjmp
  2019-09-11 18:20 ` [PATCH v3 2/3] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
@ 2019-09-11 20:55   ` Nick Desaulniers
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Desaulniers @ 2019-09-11 20:55 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: LKML, # 3.4.x, clang-built-linux, Paul Mackerras, linuxppc-dev

On Wed, Sep 11, 2019 at 11:21 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> about the setjmp and longjmp declarations.
>
> r367387 in clang added another diagnostic around this, complaining that
> there is no jmp_buf declaration.
>
> In file included from ../arch/powerpc/xmon/xmon.c:47:
> ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header <setjmp.h>.
> [-Werror,-Wincomplete-setjmp-declaration]
> extern long setjmp(long *);
>             ^
> ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header <setjmp.h>.
> [-Werror,-Wincomplete-setjmp-declaration]
> extern void longjmp(long *, long);
>             ^
> 2 errors generated.
>
> We are not using the standard library's longjmp/setjmp implementations
> for obvious reasons; make this clear to clang by using -ffreestanding
> on these files.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
https://godbolt.org/z/B2oQnl

>
> Cc: stable@vger.kernel.org # 4.14+
> Link: https://github.com/ClangBuiltLinux/linux/issues/625
> Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> v1 -> v3:
>
> * Use -ffreestanding instead of outright disabling the warning because
>   it is legitimate.
>
> I skipped v2 because the first patch in the series already had a v2.
>
>  arch/powerpc/kernel/Makefile | 4 ++--
>  arch/powerpc/xmon/Makefile   | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index c9cc4b689e60..19f19c8c874b 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -5,8 +5,8 @@
>
>  CFLAGS_ptrace.o                += -DUTS_MACHINE='"$(UTS_MACHINE)"'
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -CFLAGS_crash.o         += $(call cc-disable-warning, builtin-requires-header)
> +# Avoid clang warnings around longjmp/setjmp declarations
> +CFLAGS_crash.o         += -ffreestanding
>
>  ifdef CONFIG_PPC64
>  CFLAGS_prom_init.o     += $(NO_MINIMAL_TOC)
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index f142570ad860..c3842dbeb1b7 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -1,8 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Makefile for xmon
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
> +# Avoid clang warnings around longjmp/setjmp declarations
> +subdir-ccflags-y := -ffreestanding
>
>  GCOV_PROFILE := n
>  KCOV_INSTRUMENT := n
> --
> 2.23.0
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-09-11 18:20 ` [PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp Nathan Chancellor
@ 2019-09-11 21:01   ` Nick Desaulniers
  2019-09-12  5:43     ` Nathan Chancellor
  0 siblings, 1 reply; 21+ messages in thread
From: Nick Desaulniers @ 2019-09-11 21:01 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: LKML, clang-built-linux, Paul Mackerras, linuxppc-dev

On Wed, Sep 11, 2019 at 11:21 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> r370454 gives LLVM the ability to convert certain loops into a reference
> to bcmp as an optimization; this breaks prom_init_check.sh:
>
>   CALL    arch/powerpc/kernel/prom_init_check.sh
> Error: External symbol 'bcmp' referenced from prom_init.c
> make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1
>
> bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
> added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
> don't use string functions from lib/") copied memcmp as prom_memcmp to
> avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
> would break that assumption. Furthermore, because the compiler is the
> one that inserted bcmp, we cannot provide something like prom_bcmp.
>
> To prevent LLVM from being clever with optimizations like this, use
> -ffreestanding to tell LLVM we are not hosted so it is not free to make
> transformations like this.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/647
> Link: https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002

The above link doesn't work for me (HTTP 404).  PEBKAC?
https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002

> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> New patch in the series so no previous version.
>
>  arch/powerpc/kernel/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 19f19c8c874b..aa78b3f6271e 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -21,7 +21,7 @@ CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>  CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>  CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>
> -CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
> +CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector) -ffreestanding
>
>  ifdef CONFIG_FUNCTION_TRACER
>  # Do not trace early boot code
> --
> 2.23.0
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-09-11 21:01   ` Nick Desaulniers
@ 2019-09-12  5:43     ` Nathan Chancellor
  2019-09-12 17:30       ` Nick Desaulniers
  0 siblings, 1 reply; 21+ messages in thread
From: Nathan Chancellor @ 2019-09-12  5:43 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: LKML, clang-built-linux, Paul Mackerras, linuxppc-dev

On Wed, Sep 11, 2019 at 02:01:59PM -0700, Nick Desaulniers wrote:
> On Wed, Sep 11, 2019 at 11:21 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > r370454 gives LLVM the ability to convert certain loops into a reference
> > to bcmp as an optimization; this breaks prom_init_check.sh:
> >
> >   CALL    arch/powerpc/kernel/prom_init_check.sh
> > Error: External symbol 'bcmp' referenced from prom_init.c
> > make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1
> >
> > bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
> > added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
> > don't use string functions from lib/") copied memcmp as prom_memcmp to
> > avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
> > would break that assumption. Furthermore, because the compiler is the
> > one that inserted bcmp, we cannot provide something like prom_bcmp.
> >
> > To prevent LLVM from being clever with optimizations like this, use
> > -ffreestanding to tell LLVM we are not hosted so it is not free to make
> > transformations like this.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/647
> > Link: https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002
> 
> The above link doesn't work for me (HTTP 404).  PEBKAC?
> https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002

Not really sure how an extra 2 got added on the end of that... Must have
screwed up in vim somehow.

Link: https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e00

I can resend unless the maintainer is able to fix that up when it gets
applied.

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-09-12  5:43     ` Nathan Chancellor
@ 2019-09-12 17:30       ` Nick Desaulniers
  0 siblings, 0 replies; 21+ messages in thread
From: Nick Desaulniers @ 2019-09-12 17:30 UTC (permalink / raw)
  To: Nathan Chancellor, Michael Ellerman
  Cc: clang-built-linux, Paul Mackerras, linuxppc-dev, LKML

On Wed, Sep 11, 2019 at 10:43 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Wed, Sep 11, 2019 at 02:01:59PM -0700, Nick Desaulniers wrote:
> > On Wed, Sep 11, 2019 at 11:21 AM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > >
> > > r370454 gives LLVM the ability to convert certain loops into a reference
> > > to bcmp as an optimization; this breaks prom_init_check.sh:
> > >
> > >   CALL    arch/powerpc/kernel/prom_init_check.sh
> > > Error: External symbol 'bcmp' referenced from prom_init.c
> > > make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1
> > >
> > > bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
> > > added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
> > > don't use string functions from lib/") copied memcmp as prom_memcmp to
> > > avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
> > > would break that assumption. Furthermore, because the compiler is the
> > > one that inserted bcmp, we cannot provide something like prom_bcmp.
> > >
> > > To prevent LLVM from being clever with optimizations like this, use
> > > -ffreestanding to tell LLVM we are not hosted so it is not free to make
> > > transformations like this.
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/647
> > > Link: https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002
> >
> > The above link doesn't work for me (HTTP 404).  PEBKAC?
> > https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002
>
> Not really sure how an extra 2 got added on the end of that... Must have
> screwed up in vim somehow.
>
> Link: https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e00

That looks better.  Assuming Michael doesn't mind amending the link
when applying:
Reviewed-by: Nick Desaulneris <ndesaulniers@google.com>

>
> I can resend unless the maintainer is able to fix that up when it gets
> applied.
>
> Cheers,
> Nathan



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig
  2019-09-11 18:20 [PATCH v3 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
                   ` (2 preceding siblings ...)
  2019-09-11 18:20 ` [PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp Nathan Chancellor
@ 2019-10-14  2:50 ` Nathan Chancellor
  2019-10-14  2:50   ` [PATCH v4 1/3] powerpc: Don't add -mabi= flags when building with Clang Nathan Chancellor
                     ` (3 more replies)
  3 siblings, 4 replies; 21+ messages in thread
From: Nathan Chancellor @ 2019-10-14  2:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: clang-built-linux, Nick Desaulniers, linuxppc-dev, linux-kernel

Hi all,

This series includes a set of fixes for LLVM/Clang when building
pseries_defconfig. These have been floating around as standalone
patches so I decided to gather them up as a series so it was easier
to review/apply them.

This has been broken for a bit now, it would be nice to get these
reviewed and applied. Please let me know if I need to do anything
to move these along.

Previous versions:

https://lore.kernel.org/lkml/20190911182049.77853-1-natechancellor@gmail.com/

Cheers,
Nathan



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 1/3] powerpc: Don't add -mabi= flags when building with Clang
  2019-10-14  2:50 ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
@ 2019-10-14  2:50   ` Nathan Chancellor
  2019-10-14  2:51   ` [PATCH v4 2/3] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2019-10-14  2:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux,
	Nathan Chancellor, linuxppc-dev, Daniel Axtens

When building pseries_defconfig, building vdso32 errors out:

  error: unknown target ABI 'elfv1'

This happens because -m32 in clang changes the target to 32-bit,
which does not allow the ABI to be changed, as the setABI virtual
function is not overridden:

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/include/clang/Basic/TargetInfo.h#L1073-L1078

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Basic/Targets/PPC.h#L327-L365

Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
powerpc64le toolchain") added these flags to fix building big endian
kernels with a little endian GCC.

Clang doesn't need -mabi because the target triple controls the default
value. -mlittle-endian and -mbig-endian manipulate the triple into
either powerpc64-* or powerpc64le-*, which properly sets the default
ABI:

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Driver/Driver.cpp#L450-L463

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/llvm/lib/Support/Triple.cpp#L1432-L1516

https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Basic/Targets/PPC.h#L377-L383

Adding a debug print out in the PPC64TargetInfo constructor after line
383 above shows this:

$ echo | ./clang -E --target=powerpc64-linux -mbig-endian -o /dev/null -
Default ABI: elfv1

$ echo | ./clang -E --target=powerpc64-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2

$ echo | ./clang -E --target=powerpc64le-linux -mbig-endian -o /dev/null -
Default ABI: elfv1

$ echo | ./clang -E --target=powerpc64le-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2

Don't specify -mabi when building with clang to avoid the build error
with -m32 and not change any code generation.

-mcall-aixdesc is not an implemented flag in clang so it can be
safely excluded as well, see commit 238abecde8ad ("powerpc: Don't
use gcc specific options on clang").

pseries_defconfig successfully builds after this patch and
powernv_defconfig and ppc44x_defconfig don't regress.

Link: https://github.com/ClangBuiltLinux/linux/issues/240
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

v1 -> v2:

* Improve commit message

v2 -> v3:

* Rebase and merge into a single series.

v3 -> v4:

* Rebase on v5.4-rc3.

* Update links to point to llvmorg-9.0.0 instead of llvmorg-9.0.0-rc2.

 arch/powerpc/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 83522c9fc7b6..37ac731a556b 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -91,11 +91,13 @@ MULTIPLEWORD	:= -mmultiple
 endif
 
 ifdef CONFIG_PPC64
+ifndef CONFIG_CC_IS_CLANG
 cflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mabi=elfv1)
 cflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mcall-aixdesc)
 aflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mabi=elfv1)
 aflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mabi=elfv2
 endif
+endif
 
 ifndef CONFIG_CC_IS_CLANG
   cflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mno-strict-align
@@ -141,6 +143,7 @@ endif
 endif
 
 CFLAGS-$(CONFIG_PPC64)	:= $(call cc-option,-mtraceback=no)
+ifndef CONFIG_CC_IS_CLANG
 ifdef CONFIG_CPU_LITTLE_ENDIAN
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv2,$(call cc-option,-mcall-aixdesc))
 AFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv2)
@@ -149,6 +152,7 @@ CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv1)
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mcall-aixdesc)
 AFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv1)
 endif
+endif
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mcmodel=medium,$(call cc-option,-mminimal-toc))
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mno-pointers-to-nested-functions)
 
-- 
2.23.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 2/3] powerpc: Avoid clang warnings around setjmp and longjmp
  2019-10-14  2:50 ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
  2019-10-14  2:50   ` [PATCH v4 1/3] powerpc: Don't add -mabi= flags when building with Clang Nathan Chancellor
@ 2019-10-14  2:51   ` Nathan Chancellor
  2019-10-14  2:51   ` [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp Nathan Chancellor
  2019-10-14 16:03   ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nick Desaulniers
  3 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2019-10-14  2:51 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Nick Desaulniers, linux-kernel, stable, clang-built-linux,
	Nathan Chancellor, linuxppc-dev

Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
setjmp is used") disabled -Wbuiltin-requires-header because of a warning
about the setjmp and longjmp declarations.

r367387 in clang added another diagnostic around this, complaining that
there is no jmp_buf declaration.

In file included from ../arch/powerpc/xmon/xmon.c:47:
../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
built-in function 'setjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header <setjmp.h>.
[-Werror,-Wincomplete-setjmp-declaration]
extern long setjmp(long *);
            ^
../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
built-in function 'longjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header <setjmp.h>.
[-Werror,-Wincomplete-setjmp-declaration]
extern void longjmp(long *, long);
            ^
2 errors generated.

We are not using the standard library's longjmp/setjmp implementations
for obvious reasons; make this clear to clang by using -ffreestanding
on these files.

Cc: stable@vger.kernel.org # 4.14+
Link: https://github.com/ClangBuiltLinux/linux/issues/625
Link: https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
Link: https://godbolt.org/z/B2oQnl
Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

v1 -> v3 (I skipped v2 because the first patch in the series already
          had a v2):

* Use -ffreestanding instead of outright disabling the warning because
  it is legitimate.

v3 -> v4:

* Rebase on v5.4-rc3

* Add Nick's reviewed-by and Compiler Explorer link.

 arch/powerpc/kernel/Makefile | 4 ++--
 arch/powerpc/xmon/Makefile   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index a7ca8fe62368..f1f362146135 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -5,8 +5,8 @@
 
 CFLAGS_ptrace.o		+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
 
-# Disable clang warning for using setjmp without setjmp.h header
-CFLAGS_crash.o		+= $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+CFLAGS_crash.o		+= -ffreestanding
 
 ifdef CONFIG_PPC64
 CFLAGS_prom_init.o	+= $(NO_MINIMAL_TOC)
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index f142570ad860..c3842dbeb1b7 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for xmon
 
-# Disable clang warning for using setjmp without setjmp.h header
-subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+subdir-ccflags-y := -ffreestanding
 
 GCOV_PROFILE := n
 KCOV_INSTRUMENT := n
-- 
2.23.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-10-14  2:50 ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
  2019-10-14  2:50   ` [PATCH v4 1/3] powerpc: Don't add -mabi= flags when building with Clang Nathan Chancellor
  2019-10-14  2:51   ` [PATCH v4 2/3] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
@ 2019-10-14  2:51   ` Nathan Chancellor
  2019-10-14  9:35     ` Segher Boessenkool
  2019-10-14 16:03   ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nick Desaulniers
  3 siblings, 1 reply; 21+ messages in thread
From: Nathan Chancellor @ 2019-10-14  2:51 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: clang-built-linux, Nathan Chancellor, Nick Desaulniers,
	linuxppc-dev, linux-kernel

r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:

  CALL    arch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1

bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.

To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.

Link: https://github.com/ClangBuiltLinux/linux/issues/647
Link: https://github.com/llvm/llvm-project/commit/76cdcf25b883751d83402baea6316772aa73865c
Reviewed-by: Nick Desaulneris <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

v1 -> v3:

* New patch in the series

v3 -> v4:

* Rebase on v5.4-rc3.

* Add Nick's reviewed-by tag.

* Update the LLVM commit reference to the latest applied version (r374662)
  as it was originally committed as r370454, reverted in r370788, and
  reapplied as r374662.

 arch/powerpc/kernel/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index f1f362146135..7f0ee465dfb6 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -21,7 +21,7 @@ CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 
-CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
+CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector) -ffreestanding
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
-- 
2.23.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-10-14  2:51   ` [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp Nathan Chancellor
@ 2019-10-14  9:35     ` Segher Boessenkool
  2019-10-14 15:56       ` Nick Desaulniers
  0 siblings, 1 reply; 21+ messages in thread
From: Segher Boessenkool @ 2019-10-14  9:35 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux,
	Paul Mackerras, linuxppc-dev

On Sun, Oct 13, 2019 at 07:51:01PM -0700, Nathan Chancellor wrote:
> r374662 gives LLVM the ability to convert certain loops into a reference
> to bcmp as an optimization; this breaks prom_init_check.sh:

When/why does LLVM think this is okay?  This function has been removed
from POSIX over a decade ago (and before that it always was marked as
legacy).


Segher

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-10-14  9:35     ` Segher Boessenkool
@ 2019-10-14 15:56       ` Nick Desaulniers
  2019-10-14 19:11         ` Segher Boessenkool
  0 siblings, 1 reply; 21+ messages in thread
From: Nick Desaulniers @ 2019-10-14 15:56 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: LKML, clang-built-linux, Paul Mackerras, Nathan Chancellor, linuxppc-dev

On Mon, Oct 14, 2019 at 2:35 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Sun, Oct 13, 2019 at 07:51:01PM -0700, Nathan Chancellor wrote:
> > r374662 gives LLVM the ability to convert certain loops into a reference
> > to bcmp as an optimization; this breaks prom_init_check.sh:
>
> When/why does LLVM think this is okay?  This function has been removed
> from POSIX over a decade ago (and before that it always was marked as
> legacy).

Segher, do you have links for any of the above? If so, that would be
helpful to me. I'm arguing against certain transforms that assume that
one library function is faster than another, when such claims are
based on measurements from one stdlib implementation. (There's others
in the pipeline I'm not too thrilled about, too).

The rationale for why it was added was that memcmp takes a measurable
amount of time in Google's fleet, and most calls to memcmp don't care
about the position of the mismatch; bcmp is lower overhead (or at
least for our libc implementation, not sure about others).
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig
  2019-10-14  2:50 ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
                     ` (2 preceding siblings ...)
  2019-10-14  2:51   ` [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp Nathan Chancellor
@ 2019-10-14 16:03   ` Nick Desaulniers
  3 siblings, 0 replies; 21+ messages in thread
From: Nick Desaulniers @ 2019-10-14 16:03 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: LKML, clang-built-linux, Paul Mackerras, Nathan Chancellor, linuxppc-dev

On Sun, Oct 13, 2019 at 7:51 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Hi all,
>
> This series includes a set of fixes for LLVM/Clang when building
> pseries_defconfig. These have been floating around as standalone
> patches so I decided to gather them up as a series so it was easier
> to review/apply them.
>
> This has been broken for a bit now, it would be nice to get these
> reviewed and applied. Please let me know if I need to do anything
> to move these along.

+1, we've been carrying these out of tree for some time now, with this
series merged, we can get back to 0 out of tree patches.

>
> Previous versions:
>
> https://lore.kernel.org/lkml/20190911182049.77853-1-natechancellor@gmail.com/
>
> Cheers,
> Nathan
>
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-10-14 15:56       ` Nick Desaulniers
@ 2019-10-14 19:11         ` Segher Boessenkool
  2019-10-18 19:00           ` Nathan Chancellor
  0 siblings, 1 reply; 21+ messages in thread
From: Segher Boessenkool @ 2019-10-14 19:11 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: LKML, clang-built-linux, Paul Mackerras, Nathan Chancellor, linuxppc-dev

On Mon, Oct 14, 2019 at 08:56:12AM -0700, Nick Desaulniers wrote:
> On Mon, Oct 14, 2019 at 2:35 AM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > On Sun, Oct 13, 2019 at 07:51:01PM -0700, Nathan Chancellor wrote:
> > > r374662 gives LLVM the ability to convert certain loops into a reference
> > > to bcmp as an optimization; this breaks prom_init_check.sh:
> >
> > When/why does LLVM think this is okay?  This function has been removed
> > from POSIX over a decade ago (and before that it always was marked as
> > legacy).
> 
> Segher, do you have links for any of the above? If so, that would be
> helpful to me.

Sure!

https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap03.html

Older versions are harder to find online, unfortunately.  But there is

https://kernel.org/pub/linux/docs/man-pages/man-pages-posix/

in which man3p/bcmp.3p says:

FUTURE DIRECTIONS
       This function may be withdrawn in a future version.

Finally, the Linux man pages say (man bcmp):

CONFORMING TO
       4.3BSD.   This  function   is   deprecated   (marked   as   LEGACY   in
       POSIX.1-2001): use memcmp(3) in new programs.  POSIX.1-2008 removes the
       specification of bcmp().


> I'm arguing against certain transforms that assume that
> one library function is faster than another, when such claims are
> based on measurements from one stdlib implementation.

Wow.  The difference between memcmp and bcmp is trivial (just the return
value is different, and that costs hardly anything to add).  And memcmp
is guaranteed to exist since C89/C90 at least.

> The rationale for why it was added was that memcmp takes a measurable
> amount of time in Google's fleet, and most calls to memcmp don't care
> about the position of the mismatch; bcmp is lower overhead (or at
> least for our libc implementation, not sure about others).

You just have to do the read of the last words you compare as big-endian,
and then you can just subtract the two words, convert that to "int" (which
is very inconvenient to do, but hardly expensive), and there you go.

Or on x86 use the bswap insn, or something like it.

Or, if you use GCC, it has __builtin_memcmp but also __builtin_memcmp_eq,
and those are automatically used, too.


Segher

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-10-14 19:11         ` Segher Boessenkool
@ 2019-10-18 19:00           ` Nathan Chancellor
  2019-10-18 20:02             ` Segher Boessenkool
  0 siblings, 1 reply; 21+ messages in thread
From: Nathan Chancellor @ 2019-10-18 19:00 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Nick Desaulniers, LKML, clang-built-linux, Paul Mackerras, linuxppc-dev

On Mon, Oct 14, 2019 at 02:11:41PM -0500, Segher Boessenkool wrote:
> On Mon, Oct 14, 2019 at 08:56:12AM -0700, Nick Desaulniers wrote:
> > On Mon, Oct 14, 2019 at 2:35 AM Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> > >
> > > On Sun, Oct 13, 2019 at 07:51:01PM -0700, Nathan Chancellor wrote:
> > > > r374662 gives LLVM the ability to convert certain loops into a reference
> > > > to bcmp as an optimization; this breaks prom_init_check.sh:
> > >
> > > When/why does LLVM think this is okay?  This function has been removed
> > > from POSIX over a decade ago (and before that it always was marked as
> > > legacy).
> > 
> > Segher, do you have links for any of the above? If so, that would be
> > helpful to me.
> 
> Sure!
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap03.html
> 
> Older versions are harder to find online, unfortunately.  But there is
> 
> https://kernel.org/pub/linux/docs/man-pages/man-pages-posix/
> 
> in which man3p/bcmp.3p says:
> 
> FUTURE DIRECTIONS
>        This function may be withdrawn in a future version.
> 
> Finally, the Linux man pages say (man bcmp):
> 
> CONFORMING TO
>        4.3BSD.   This  function   is   deprecated   (marked   as   LEGACY   in
>        POSIX.1-2001): use memcmp(3) in new programs.  POSIX.1-2008 removes the
>        specification of bcmp().
> 
> 
> > I'm arguing against certain transforms that assume that
> > one library function is faster than another, when such claims are
> > based on measurements from one stdlib implementation.
> 
> Wow.  The difference between memcmp and bcmp is trivial (just the return
> value is different, and that costs hardly anything to add).  And memcmp
> is guaranteed to exist since C89/C90 at least.
> 
> > The rationale for why it was added was that memcmp takes a measurable
> > amount of time in Google's fleet, and most calls to memcmp don't care
> > about the position of the mismatch; bcmp is lower overhead (or at
> > least for our libc implementation, not sure about others).
> 
> You just have to do the read of the last words you compare as big-endian,
> and then you can just subtract the two words, convert that to "int" (which
> is very inconvenient to do, but hardly expensive), and there you go.
> 
> Or on x86 use the bswap insn, or something like it.
> 
> Or, if you use GCC, it has __builtin_memcmp but also __builtin_memcmp_eq,
> and those are automatically used, too.
> 
> 
> Segher

Just as an FYI, there was some more discussion around the availablity
and use of bcmp in this LLVM bug which spawned
commit 5f074f3e192f ("lib/string.c: implement a basic bcmp").

https://bugs.llvm.org/show_bug.cgi?id=41035#c13

I believe this is the proper solution but I am fine with whatever works,
I just want our CI to be green without any out of tree patches again...

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-10-18 19:00           ` Nathan Chancellor
@ 2019-10-18 20:02             ` Segher Boessenkool
  2019-10-22  5:15               ` Nathan Chancellor
  0 siblings, 1 reply; 21+ messages in thread
From: Segher Boessenkool @ 2019-10-18 20:02 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Nick Desaulniers, LKML, clang-built-linux, Paul Mackerras, linuxppc-dev

On Fri, Oct 18, 2019 at 12:00:22PM -0700, Nathan Chancellor wrote:
> Just as an FYI, there was some more discussion around the availablity
> and use of bcmp in this LLVM bug which spawned
> commit 5f074f3e192f ("lib/string.c: implement a basic bcmp").
> 
> https://bugs.llvm.org/show_bug.cgi?id=41035#c13
> 
> I believe this is the proper solution but I am fine with whatever works,
> I just want our CI to be green without any out of tree patches again...

I think the proper solution is for the kernel to *do* use -ffreestanding,
and then somehow tell the kernel that memcpy etc. are the standard
functions.  A freestanding GCC already requires memcpy, memmove, memset,
memcmp, and sometimes abort to exist and do the standard thing; why cannot
programs then also rely on it to be the standard functions.

What exact functions are the reason the kernel does not use -ffreestanding?
Is it just memcpy?  Is more wanted?


Segher

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-10-18 20:02             ` Segher Boessenkool
@ 2019-10-22  5:15               ` Nathan Chancellor
  2019-10-22  8:57                 ` Segher Boessenkool
  0 siblings, 1 reply; 21+ messages in thread
From: Nathan Chancellor @ 2019-10-22  5:15 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Nick Desaulniers, LKML, clang-built-linux, Paul Mackerras, linuxppc-dev

On Fri, Oct 18, 2019 at 03:02:10PM -0500, Segher Boessenkool wrote:
> On Fri, Oct 18, 2019 at 12:00:22PM -0700, Nathan Chancellor wrote:
> > Just as an FYI, there was some more discussion around the availablity
> > and use of bcmp in this LLVM bug which spawned
> > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp").
> > 
> > https://bugs.llvm.org/show_bug.cgi?id=41035#c13
> > 
> > I believe this is the proper solution but I am fine with whatever works,
> > I just want our CI to be green without any out of tree patches again...
> 
> I think the proper solution is for the kernel to *do* use -ffreestanding,
> and then somehow tell the kernel that memcpy etc. are the standard
> functions.  A freestanding GCC already requires memcpy, memmove, memset,
> memcmp, and sometimes abort to exist and do the standard thing; why cannot
> programs then also rely on it to be the standard functions.
> 
> What exact functions are the reason the kernel does not use -ffreestanding?
> Is it just memcpy?  Is more wanted?
> 
> 
> Segher

I think Linus summarized it pretty well here:

https://lore.kernel.org/lkml/CAHk-=wi-epJZfBHDbKKDZ64us7WkF=LpUfhvYBmZSteO8Q0RAg@mail.gmail.com/

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-10-22  5:15               ` Nathan Chancellor
@ 2019-10-22  8:57                 ` Segher Boessenkool
  2019-10-30  4:12                   ` Nathan Chancellor
  0 siblings, 1 reply; 21+ messages in thread
From: Segher Boessenkool @ 2019-10-22  8:57 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Nick Desaulniers, LKML, clang-built-linux, Paul Mackerras, linuxppc-dev

On Mon, Oct 21, 2019 at 10:15:29PM -0700, Nathan Chancellor wrote:
> On Fri, Oct 18, 2019 at 03:02:10PM -0500, Segher Boessenkool wrote:
> > I think the proper solution is for the kernel to *do* use -ffreestanding,
> > and then somehow tell the kernel that memcpy etc. are the standard
> > functions.  A freestanding GCC already requires memcpy, memmove, memset,
> > memcmp, and sometimes abort to exist and do the standard thing; why cannot
> > programs then also rely on it to be the standard functions.
> > 
> > What exact functions are the reason the kernel does not use -ffreestanding?
> > Is it just memcpy?  Is more wanted?
> 
> I think Linus summarized it pretty well here:
> 
> https://lore.kernel.org/lkml/CAHk-=wi-epJZfBHDbKKDZ64us7WkF=LpUfhvYBmZSteO8Q0RAg@mail.gmail.com/

GCC recognises __builtin_memcpy (or any other __builtin) just fine even
with -ffreestanding.

So the kernel wants a warning (or error) whenever a call to one of these
library functions is generated by the compiler without the user asking
for it directly (via a __builtin)?  And that is all that is needed for
the kernel to use -ffreestanding?

That shouldn't be hard.  Anything missing here?


Segher

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
  2019-10-22  8:57                 ` Segher Boessenkool
@ 2019-10-30  4:12                   ` Nathan Chancellor
  0 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2019-10-30  4:12 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Nick Desaulniers, LKML, clang-built-linux, Paul Mackerras, linuxppc-dev

On Tue, Oct 22, 2019 at 03:57:09AM -0500, Segher Boessenkool wrote:
> On Mon, Oct 21, 2019 at 10:15:29PM -0700, Nathan Chancellor wrote:
> > On Fri, Oct 18, 2019 at 03:02:10PM -0500, Segher Boessenkool wrote:
> > > I think the proper solution is for the kernel to *do* use -ffreestanding,
> > > and then somehow tell the kernel that memcpy etc. are the standard
> > > functions.  A freestanding GCC already requires memcpy, memmove, memset,
> > > memcmp, and sometimes abort to exist and do the standard thing; why cannot
> > > programs then also rely on it to be the standard functions.
> > > 
> > > What exact functions are the reason the kernel does not use -ffreestanding?
> > > Is it just memcpy?  Is more wanted?
> > 
> > I think Linus summarized it pretty well here:
> > 
> > https://lore.kernel.org/lkml/CAHk-=wi-epJZfBHDbKKDZ64us7WkF=LpUfhvYBmZSteO8Q0RAg@mail.gmail.com/
> 
> GCC recognises __builtin_memcpy (or any other __builtin) just fine even
> with -ffreestanding.
> 
> So the kernel wants a warning (or error) whenever a call to one of these
> library functions is generated by the compiler without the user asking
> for it directly (via a __builtin)?  And that is all that is needed for
> the kernel to use -ffreestanding?
> 
> That shouldn't be hard.  Anything missing here?
> 
> 
> Segher

Yes, I suppose that would be good enough.

I don't know if there are any other optimizations that are missed out on
by using -ffreestanding. It would probably be worth asking other kernel
developers on a separate thread (or the one I linked above).

Would be nice to get this shored up soon since our PowerPC builds have
been broken since the beginning of August :/

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, back to index

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11 18:20 [PATCH v3 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
2019-09-11 18:20 ` [PATCH v3 1/3] powerpc: Don't add -mabi= flags when building with Clang Nathan Chancellor
2019-09-11 18:20 ` [PATCH v3 2/3] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
2019-09-11 20:55   ` Nick Desaulniers
2019-09-11 18:20 ` [PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp Nathan Chancellor
2019-09-11 21:01   ` Nick Desaulniers
2019-09-12  5:43     ` Nathan Chancellor
2019-09-12 17:30       ` Nick Desaulniers
2019-10-14  2:50 ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nathan Chancellor
2019-10-14  2:50   ` [PATCH v4 1/3] powerpc: Don't add -mabi= flags when building with Clang Nathan Chancellor
2019-10-14  2:51   ` [PATCH v4 2/3] powerpc: Avoid clang warnings around setjmp and longjmp Nathan Chancellor
2019-10-14  2:51   ` [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp Nathan Chancellor
2019-10-14  9:35     ` Segher Boessenkool
2019-10-14 15:56       ` Nick Desaulniers
2019-10-14 19:11         ` Segher Boessenkool
2019-10-18 19:00           ` Nathan Chancellor
2019-10-18 20:02             ` Segher Boessenkool
2019-10-22  5:15               ` Nathan Chancellor
2019-10-22  8:57                 ` Segher Boessenkool
2019-10-30  4:12                   ` Nathan Chancellor
2019-10-14 16:03   ` [PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig Nick Desaulniers

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git