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-L1078https://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-L463https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/llvm/lib/Support/Triple.cpp#L1432-L1516https://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 <redacted>
Signed-off-by: Nathan Chancellor <redacted>
---
v1 -> v2:
* Improve commit message
v2 -> v3:
* Rebase and merge into a single series.
arch/powerpc/Makefile | 4 ++++
1 file changed, 4 insertions(+)
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 <redacted>
Signed-off-by: Nathan Chancellor <redacted>
---
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(-)
@@ -5,8 +5,8 @@CFLAGS_ptrace.o+=-DUTS_MACHINE='"$(UTS_MACHINE)"'-# Disable clang warning for using setjmp without setjmp.h header-CFLAGS_crash.o+=$(callcc-disable-warning,builtin-requires-header)+# Avoid clang warnings around longjmp/setjmp declarations+CFLAGS_crash.o+=-ffreestandingifdef CONFIG_PPC64CFLAGS_prom_init.o+=$(NO_MINIMAL_TOC)
@@ -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:=$(callcc-disable-warning,builtin-requires-header)+# Avoid clang warnings around longjmp/setjmp declarations+subdir-ccflags-y:=-ffreestandingGCOV_PROFILE:=nKCOV_INSTRUMENT:=n
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 <redacted>
---
New patch in the series so no previous version.
arch/powerpc/kernel/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -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+=$(callcc-option,-fno-stack-protector)+CFLAGS_prom_init.o+=$(callcc-option,-fno-stack-protector)-ffreestandingifdef CONFIG_FUNCTION_TRACER# Do not trace early boot code
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2019-09-11 20:56:05
On Wed, Sep 11, 2019 at 11:21 AM Nathan Chancellor
[off-list ref] 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.
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 <redacted>
Signed-off-by: Nathan Chancellor <redacted>
---
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(-)
@@ -5,8 +5,8 @@CFLAGS_ptrace.o+=-DUTS_MACHINE='"$(UTS_MACHINE)"'-# Disable clang warning for using setjmp without setjmp.h header-CFLAGS_crash.o+=$(callcc-disable-warning,builtin-requires-header)+# Avoid clang warnings around longjmp/setjmp declarations+CFLAGS_crash.o+=-ffreestandingifdef CONFIG_PPC64CFLAGS_prom_init.o+=$(NO_MINIMAL_TOC)
@@ -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:=$(callcc-disable-warning,builtin-requires-header)+# Avoid clang warnings around longjmp/setjmp declarations+subdir-ccflags-y:=-ffreestandingGCOV_PROFILE:=nKCOV_INSTRUMENT:=n--
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2019-09-11 21:02:16
On Wed, Sep 11, 2019 at 11:21 AM Nathan Chancellor
[off-list ref] 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
Signed-off-by: Nathan Chancellor <redacted>
---
New patch in the series so no previous version.
arch/powerpc/kernel/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -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+=$(callcc-option,-fno-stack-protector)+CFLAGS_prom_init.o+=$(callcc-option,-fno-stack-protector)-ffreestandingifdef CONFIG_FUNCTION_TRACER# Do not trace early boot code--
On Wed, Sep 11, 2019 at 02:01:59PM -0700, Nick Desaulniers wrote:
On Wed, Sep 11, 2019 at 11:21 AM Nathan Chancellor
[off-list ref] wrote:
quoted
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
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2019-09-12 17:31:11
On Wed, Sep 11, 2019 at 10:43 PM Nathan Chancellor
[off-list ref] wrote:
On Wed, Sep 11, 2019 at 02:01:59PM -0700, Nick Desaulniers wrote:
quoted
On Wed, Sep 11, 2019 at 11:21 AM Nathan Chancellor
[off-list ref] wrote:
quoted
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
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
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 <redacted>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <redacted>
---
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(-)
@@ -5,8 +5,8 @@CFLAGS_ptrace.o+=-DUTS_MACHINE='"$(UTS_MACHINE)"'-# Disable clang warning for using setjmp without setjmp.h header-CFLAGS_crash.o+=$(callcc-disable-warning,builtin-requires-header)+# Avoid clang warnings around longjmp/setjmp declarations+CFLAGS_crash.o+=-ffreestandingifdef CONFIG_PPC64CFLAGS_prom_init.o+=$(NO_MINIMAL_TOC)
@@ -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:=$(callcc-disable-warning,builtin-requires-header)+# Avoid clang warnings around longjmp/setjmp declarations+subdir-ccflags-y:=-ffreestandingGCOV_PROFILE:=nKCOV_INSTRUMENT:=n
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 <redacted>
---
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(-)
@@ -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+=$(callcc-option,-fno-stack-protector)+CFLAGS_prom_init.o+=$(callcc-option,-fno-stack-protector)-ffreestandingifdef CONFIG_FUNCTION_TRACER# Do not trace early boot code
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-L1078https://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-L463https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/llvm/lib/Support/Triple.cpp#L1432-L1516https://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 <redacted>
Signed-off-by: Nathan Chancellor <redacted>
---
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(+)
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
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2019-10-14 15:56:26
On Mon, Oct 14, 2019 at 2:35 AM Segher Boessenkool
[off-list ref] wrote:
On Sun, Oct 13, 2019 at 07:51:01PM -0700, Nathan Chancellor wrote:
quoted
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
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2019-10-14 16:03:43
On Sun, Oct 13, 2019 at 7:51 PM Nathan Chancellor
[off-list ref] 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.
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
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.
quoted
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
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
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:
quoted
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
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:
quoted
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?
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
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:
quoted
On Fri, Oct 18, 2019 at 03:02:10PM -0500, Segher Boessenkool wrote:
quoted
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?
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
Hi all,
This series includes a set of fixes for LLVM/Clang when building
a few defconfigs (powernv, ppc44x, and pseries are the ones that our
CI configuration tests [1]). The first patch fixes pseries_defconfig,
which has never worked in mainline. The second and third patches fixes
issues with all of these configs due to internal changes to LLVM, which
point out issues with the kernel.
These have been broken since July/August, it would be nice to get these
reviewed and applied. Please let me know what I can do to get these
applied soon so we can stop applying them out of tree.
[1]: https://github.com/ClangBuiltLinux/continuous-integration
Previous versions:
v3: https://lore.kernel.org/lkml/20190911182049.77853-1-natechancellor@gmail.com/
v4: https://lore.kernel.org/lkml/20191014025101.18567-1-natechancellor@gmail.com/
Cheers,
Nathan
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-L1078https://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-L463https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/llvm/lib/Support/Triple.cpp#L1432-L1516https://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 <redacted>
Signed-off-by: Nathan Chancellor <redacted>
---
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.
v4 -> v5:
* Rebase on next-20191118
arch/powerpc/Makefile | 4 ++++
1 file changed, 4 insertions(+)
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 <redacted>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <redacted>
---
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.
v4 -> v5:
* Rebase on next-20191118
arch/powerpc/kernel/Makefile | 4 ++--
arch/powerpc/xmon/Makefile | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
@@ -5,8 +5,8 @@CFLAGS_ptrace.o+=-DUTS_MACHINE='"$(UTS_MACHINE)"'-# Disable clang warning for using setjmp without setjmp.h header-CFLAGS_crash.o+=$(callcc-disable-warning,builtin-requires-header)+# Avoid clang warnings around longjmp/setjmp declarations+CFLAGS_crash.o+=-ffreestandingifdef CONFIG_PPC64CFLAGS_prom_init.o+=$(NO_MINIMAL_TOC)
@@ -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:=$(callcc-disable-warning,builtin-requires-header)+# Avoid clang warnings around longjmp/setjmp declarations+subdir-ccflags-y:=-ffreestandingGCOV_PROFILE:=nKCOV_INSTRUMENT:=n
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 <redacted>
---
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.
v4 -> v5:
* Rebase on next-20191118 to avoid a conflict with commit
6266a4dadb1d ("powerpc/64s: Always disable branch profiling for prom_init.o")
arch/powerpc/kernel/Makefile | 1 +
1 file changed, 1 insertion(+)
@@ -23,6 +23,7 @@ CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)CFLAGS_prom_init.o+=$(callcc-option,-fno-stack-protector)CFLAGS_prom_init.o+=-DDISABLE_BRANCH_PROFILING+CFLAGS_prom_init.o+=-ffreestandingifdef CONFIG_FUNCTION_TRACER# Do not trace early boot code
From: Nick Desaulniers <ndesaulniers@google.com> Date: 2019-11-25 20:15:30
Hi Michael,
Do you have feedback for Nathan? Rebasing these patches is becoming a
nuisance for our CI, and we would like to keep building PPC w/ Clang.
On Mon, Nov 18, 2019 at 8:57 PM Nathan Chancellor
[off-list ref] wrote:
Hi all,
This series includes a set of fixes for LLVM/Clang when building
a few defconfigs (powernv, ppc44x, and pseries are the ones that our
CI configuration tests [1]). The first patch fixes pseries_defconfig,
which has never worked in mainline. The second and third patches fixes
issues with all of these configs due to internal changes to LLVM, which
point out issues with the kernel.
These have been broken since July/August, it would be nice to get these
reviewed and applied. Please let me know what I can do to get these
applied soon so we can stop applying them out of tree.
[1]: https://github.com/ClangBuiltLinux/continuous-integration
Previous versions:
v3: https://lore.kernel.org/lkml/20190911182049.77853-1-natechancellor@gmail.com/
v4: https://lore.kernel.org/lkml/20191014025101.18567-1-natechancellor@gmail.com/
Cheers,
Nathan
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2019-11-28 04:59:15
Nick Desaulniers [off-list ref] writes:
Hi Michael,
Do you have feedback for Nathan? Rebasing these patches is becoming a
nuisance for our CI, and we would like to keep building PPC w/ Clang.
Sorry just lost in the flood of patches.
Merged now.
cheers
On Mon, Nov 18, 2019 at 8:57 PM Nathan Chancellor
[off-list ref] wrote:
quoted
Hi all,
This series includes a set of fixes for LLVM/Clang when building
a few defconfigs (powernv, ppc44x, and pseries are the ones that our
CI configuration tests [1]). The first patch fixes pseries_defconfig,
which has never worked in mainline. The second and third patches fixes
issues with all of these configs due to internal changes to LLVM, which
point out issues with the kernel.
These have been broken since July/August, it would be nice to get these
reviewed and applied. Please let me know what I can do to get these
applied soon so we can stop applying them out of tree.
[1]: https://github.com/ClangBuiltLinux/continuous-integration
Previous versions:
v3: https://lore.kernel.org/lkml/20190911182049.77853-1-natechancellor@gmail.com/
v4: https://lore.kernel.org/lkml/20191014025101.18567-1-natechancellor@gmail.com/
Cheers,
Nathan
On Thu, Nov 28, 2019 at 03:59:07PM +1100, Michael Ellerman wrote:
Nick Desaulniers [off-list ref] writes:
quoted
Hi Michael,
Do you have feedback for Nathan? Rebasing these patches is becoming a
nuisance for our CI, and we would like to keep building PPC w/ Clang.
Sorry just lost in the flood of patches.
Merged now.
cheers
Thank you very much for picking them up :)
Cheers,
Nathan