All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] powerpc: Clang build fixes
@ 2018-09-14  4:06 Joel Stanley
  2018-09-14  4:06 ` [PATCH v2 1/5] powerpc/Makefiles: Fix clang/llvm build Joel Stanley
                   ` (4 more replies)
  0 siblings, 5 replies; 26+ messages in thread
From: Joel Stanley @ 2018-09-14  4:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anton Blanchard, Nick Desaulniers

v2 rolls up all of the fixes I have into the one series, and adds a few
more fixes.

With a one patch[1] on top of clang master I can build and boot a
powernv kernel. Note that we can't build altivec code using clang
yet[2], hence disabling MD_RAID456.

$ make ARCH=powerpc powernv_defconfig
$ ./scripts/config -d BTRFS_FS -d MD_RAID456
$ make CC=clang-8 CLANG_TRIPLE=powerpc64le-linux-gnu -j128

The build is very noisy as the ftrace recordmcount build step is not
picking up -Qunused-arguments.

$ qemu-system-ppc64 -M powernv -m 3G -nographic -kernel zImage.epapr \
 -L ~/skiboot/ -initrd ~/rootfs.cpio.xz

Linux version 4.19.0-rc3-00005-g7f51dcdecc6b (joel@ozrom3) (clang version 8.0.0 (trunk 342199)) #5 SMP Fri Sep 14 13:28:57 ACST 2018

I have discovered Nick D's clangBuiltLinux issue tracker[3] and will be
updating that as we find and fix issues.

[1] https://reviews.llvm.org/D50965
[2] https://bugs.llvm.org/show_bug.cgi?id=38887
[3] https://github.com/clangBuiltLinux/linux/issues

Anton Blanchard (2):
  powerpc/Makefiles: Fix clang/llvm build
  powerpc: Fix duplicate const clang warning in user access code

Joel Stanley (3):
  powerpc/boot: Fix crt0.S syntax for clang
  powerpc/boot: Ensure _zimage_start is a weak symbol
  powerpc: Remove -mno-sched-epilog

 arch/powerpc/Makefile                    | 7 +------
 arch/powerpc/boot/crt0.S                 | 8 +++++---
 arch/powerpc/include/asm/uaccess.h       | 6 +++---
 arch/powerpc/kernel/Makefile             | 8 ++++----
 arch/powerpc/kernel/trace/Makefile       | 2 +-
 arch/powerpc/platforms/powermac/Makefile | 2 +-
 arch/powerpc/xmon/Makefile               | 2 +-
 7 files changed, 16 insertions(+), 19 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v2 1/5] powerpc/Makefiles: Fix clang/llvm build
  2018-09-14  4:06 [PATCH v2 0/5] powerpc: Clang build fixes Joel Stanley
@ 2018-09-14  4:06 ` Joel Stanley
  2018-09-14 17:41   ` Nick Desaulniers
  2018-09-14  4:06 ` [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang Joel Stanley
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Joel Stanley @ 2018-09-14  4:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anton Blanchard, Anton Blanchard, Nick Desaulniers

From: Anton Blanchard <anton@samba.org>

Commit 15a3204d24a3 ("powerpc/64s: Set assembler machine type to POWER4")
passes -mpower4 to the assembler. We have more recent instructions in our
assembly files, but gas permits them. The clang/llvm integrated assembler
is more strict, and we get a build failure.

Fix this by calling the assembler with -mcpu=power8 if as supports it,
else fall back to power4.

Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/powerpc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 11a1acba164a..a70639482053 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -238,7 +238,7 @@ cpu-as-$(CONFIG_4xx)		+= -Wa,-m405
 cpu-as-$(CONFIG_ALTIVEC)	+= $(call as-option,-Wa$(comma)-maltivec)
 cpu-as-$(CONFIG_E200)		+= -Wa,-me200
 cpu-as-$(CONFIG_E500)		+= -Wa,-me500
-cpu-as-$(CONFIG_PPC_BOOK3S_64)	+= -Wa,-mpower4
+cpu-as-$(CONFIG_PPC_BOOK3S_64)	+= $(call as-option,-Wa$(comma)-mpower8,-Wa$(comma)-mpower4)
 cpu-as-$(CONFIG_PPC_E500MC)	+= $(call as-option,-Wa$(comma)-me500mc)
 
 KBUILD_AFLAGS += $(cpu-as-y)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang
  2018-09-14  4:06 [PATCH v2 0/5] powerpc: Clang build fixes Joel Stanley
  2018-09-14  4:06 ` [PATCH v2 1/5] powerpc/Makefiles: Fix clang/llvm build Joel Stanley
@ 2018-09-14  4:06 ` Joel Stanley
  2018-09-14 17:47   ` Nick Desaulniers
  2018-09-14  4:06 ` [PATCH v2 3/5] powerpc/boot: Ensure _zimage_start is a weak symbol Joel Stanley
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Joel Stanley @ 2018-09-14  4:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anton Blanchard, Nick Desaulniers

Clang's assembler does not like the syntax of the cmpdi:

 arch/powerpc/boot/crt0.S:168:22: error: unexpected modifier on variable reference
         cmpdi   12,RELACOUNT@l
                              ^
 arch/powerpc/boot/crt0.S:168:11: error: unknown operand
         cmpdi   12,RELACOUNT@l
                   ^
Enclosing RELACOUNT in () makes it is happy. Tested with GCC 8 and Clang
8 (trunk).

Reported to clang as https://bugs.llvm.org/show_bug.cgi?id=38945

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v2: Fix for !powerpc64 too, add bug link to commit message
---
 arch/powerpc/boot/crt0.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
index dcf2f15e6797..e453e011d7e7 100644
--- a/arch/powerpc/boot/crt0.S
+++ b/arch/powerpc/boot/crt0.S
@@ -80,7 +80,7 @@ p_base:	mflr	r10		/* r10 now points to runtime addr of p_base */
 	lwz	r9,4(r12)	/* get RELA pointer in r9 */
 	b	12f
 11:	addis	r8,r8,(-RELACOUNT)@ha
-	cmpwi	r8,RELACOUNT@l
+	cmpwi	r8,(RELACOUNT)@l
 	bne	12f
 	lwz	r0,4(r12)	/* get RELACOUNT value in r0 */
 12:	addi	r12,r12,8
@@ -165,7 +165,7 @@ p_base:	mflr	r10		/* r10 now points to runtime addr of p_base */
 	ld	r13,8(r11)       /* get RELA pointer in r13 */
 	b	11f
 10:	addis	r12,r12,(-RELACOUNT)@ha
-	cmpdi	r12,RELACOUNT@l
+	cmpdi	r12,(RELACOUNT)@l
 	bne	11f
 	ld	r8,8(r11)       /* get RELACOUNT value in r8 */
 11:	addi	r11,r11,16
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v2 3/5] powerpc/boot: Ensure _zimage_start is a weak symbol
  2018-09-14  4:06 [PATCH v2 0/5] powerpc: Clang build fixes Joel Stanley
  2018-09-14  4:06 ` [PATCH v2 1/5] powerpc/Makefiles: Fix clang/llvm build Joel Stanley
  2018-09-14  4:06 ` [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang Joel Stanley
@ 2018-09-14  4:06 ` Joel Stanley
  2018-09-14 17:50   ` Nick Desaulniers
  2018-09-20  4:21   ` [v2,3/5] " Michael Ellerman
  2018-09-14  4:06 ` [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code Joel Stanley
  2018-09-14  4:06 ` [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog Joel Stanley
  4 siblings, 2 replies; 26+ messages in thread
From: Joel Stanley @ 2018-09-14  4:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anton Blanchard, Nick Desaulniers

When building with clang crt0's _zimage_start is not marked weak, which
breaks the build when linking the kernel image:

 $ objdump -t arch/powerpc/boot/crt0.o |grep _zimage_start$
 0000000000000058 g       .text  0000000000000000 _zimage_start

 ld: arch/powerpc/boot/wrapper.a(crt0.o): in function '_zimage_start':
 (.text+0x58): multiple definition of '_zimage_start';
 arch/powerpc/boot/pseries-head.o:(.text+0x0): first defined here

Clang requires the .weak directive to appear after the symbol is
declared. The binutils manual says:

 This directive sets the weak attribute on the comma separated list of
 symbol names. If the symbols do not already exist, they will be
 created.

So it appears this is different with clang. The only reference I could
see for this was an OpenBSD mailing list post[1].

Changing it to be after the declaration fixes building with Clang, and
still works with GCC.

 $ objdump -t arch/powerpc/boot/crt0.o |grep _zimage_start$
 0000000000000058  w      .text	0000000000000000 _zimage_start

Reported to clang as https://bugs.llvm.org/show_bug.cgi?id=38921

[1] https://groups.google.com/forum/#!topic/fa.openbsd.tech/PAgKKen2YCY

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v2: Add comment explaining this is a workaround for clang. I have also
opened a clang bug to see if they plan on changing behaviour
---
 arch/powerpc/boot/crt0.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
index e453e011d7e7..4e32bd251b99 100644
--- a/arch/powerpc/boot/crt0.S
+++ b/arch/powerpc/boot/crt0.S
@@ -47,8 +47,10 @@ p_end:		.long	_end
 p_pstack:	.long	_platform_stack_top
 #endif
 
-	.weak	_zimage_start
 	.globl	_zimage_start
+	/* Clang appears to require the .weak directive to be after the symbol
+	 * is defined. See https://bugs.llvm.org/show_bug.cgi?id=38921  */
+	.weak	_zimage_start
 _zimage_start:
 	.globl	_zimage_start_lib
 _zimage_start_lib:
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code
  2018-09-14  4:06 [PATCH v2 0/5] powerpc: Clang build fixes Joel Stanley
                   ` (2 preceding siblings ...)
  2018-09-14  4:06 ` [PATCH v2 3/5] powerpc/boot: Ensure _zimage_start is a weak symbol Joel Stanley
@ 2018-09-14  4:06 ` Joel Stanley
  2018-09-14 17:57   ` Nick Desaulniers
  2018-09-20  4:21   ` [v2, " Michael Ellerman
  2018-09-14  4:06 ` [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog Joel Stanley
  4 siblings, 2 replies; 26+ messages in thread
From: Joel Stanley @ 2018-09-14  4:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anton Blanchard, Anton Blanchard, Nick Desaulniers

From: Anton Blanchard <anton@samba.org>

This re-applies b91c1e3e7a6f which was reverted in f2ca80905929
d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer
...").

We see a large number of duplicate const errors in the user access
code when building with llvm/clang:

  include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier
      [-Wduplicate-decl-specifier]
        ret = __get_user(c, uaddr);

The problem is we are doing const __typeof__(*(ptr)), which will hit the
warning if ptr is marked const.

Removing const does not seem to have any effect on GCC code generation.

Signed-off-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
If we don't want to apply this, other options are suppressing the
warning, or wait for a fix to land in clang
(https://github.com/ClangBuiltLinux/linux/issues/52).
---
 arch/powerpc/include/asm/uaccess.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index bac225bb7f64..15bea9a0f260 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -260,7 +260,7 @@ do {								\
 ({								\
 	long __gu_err;						\
 	__long_type(*(ptr)) __gu_val;				\
-	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
+	__typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
 	__chk_user_ptr(ptr);					\
 	if (!is_kernel_addr((unsigned long)__gu_addr))		\
 		might_fault();					\
@@ -274,7 +274,7 @@ do {								\
 ({									\
 	long __gu_err = -EFAULT;					\
 	__long_type(*(ptr)) __gu_val = 0;				\
-	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);		\
+	__typeof__(*(ptr)) __user *__gu_addr = (ptr);		\
 	might_fault();							\
 	if (access_ok(VERIFY_READ, __gu_addr, (size))) {		\
 		barrier_nospec();					\
@@ -288,7 +288,7 @@ do {								\
 ({								\
 	long __gu_err;						\
 	__long_type(*(ptr)) __gu_val;				\
-	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
+	__typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
 	__chk_user_ptr(ptr);					\
 	barrier_nospec();					\
 	__get_user_size(__gu_val, __gu_addr, (size), __gu_err);	\
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog
  2018-09-14  4:06 [PATCH v2 0/5] powerpc: Clang build fixes Joel Stanley
                   ` (3 preceding siblings ...)
  2018-09-14  4:06 ` [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code Joel Stanley
@ 2018-09-14  4:06 ` Joel Stanley
  2018-09-14  5:06   ` Nicholas Piggin
  4 siblings, 1 reply; 26+ messages in thread
From: Joel Stanley @ 2018-09-14  4:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anton Blanchard, Nick Desaulniers

This effectively reverts 7563dc645853 ("powerpc: Work around gcc's
-fno-omit-frame-pointer bug"), a workaround for a bug in GCC 4.1.3 when
building 2.6.26 kernel.

The flag is not supported by clang, but reading the history of the
bug[1] suggests it is no longer required by supported GCC versions, with
GCC 4.6 now being the minimum.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=11414
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
Last time this was proposed there was an issue reported:

 https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-September/121214.html

If some machines still have an issue, we could instead wrap this in a
flag check.
---
 arch/powerpc/Makefile                    | 5 -----
 arch/powerpc/kernel/Makefile             | 8 ++++----
 arch/powerpc/kernel/trace/Makefile       | 2 +-
 arch/powerpc/platforms/powermac/Makefile | 2 +-
 arch/powerpc/xmon/Makefile               | 2 +-
 5 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index a70639482053..ef8eca40ffef 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -229,11 +229,6 @@ ifdef CONFIG_6xx
 KBUILD_CFLAGS		+= -mcpu=powerpc
 endif
 
-# Work around a gcc code-gen bug with -fno-omit-frame-pointer.
-ifdef CONFIG_FUNCTION_TRACER
-KBUILD_CFLAGS		+= -mno-sched-epilog
-endif
-
 cpu-as-$(CONFIG_4xx)		+= -Wa,-m405
 cpu-as-$(CONFIG_ALTIVEC)	+= $(call as-option,-Wa$(comma)-maltivec)
 cpu-as-$(CONFIG_E200)		+= -Wa,-me200
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 3b66f2c19c84..1e64cfe22a83 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -22,10 +22,10 @@ CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
-CFLAGS_REMOVE_cputable.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
-CFLAGS_REMOVE_prom_init.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
-CFLAGS_REMOVE_btext.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
-CFLAGS_REMOVE_prom.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_cputable.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_prom_init.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_btext.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_prom.o = $(CC_FLAGS_FTRACE)
 endif
 
 obj-y				:= cputable.o ptrace.o syscalls.o \
diff --git a/arch/powerpc/kernel/trace/Makefile b/arch/powerpc/kernel/trace/Makefile
index d22d8bafb643..d868ba42032f 100644
--- a/arch/powerpc/kernel/trace/Makefile
+++ b/arch/powerpc/kernel/trace/Makefile
@@ -7,7 +7,7 @@ subdir-ccflags-$(CONFIG_PPC_WERROR)	:= -Werror
 
 ifdef CONFIG_FUNCTION_TRACER
 # do not trace tracer code
-CFLAGS_REMOVE_ftrace.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
 endif
 
 obj32-$(CONFIG_FUNCTION_TRACER)		+= ftrace_32.o
diff --git a/arch/powerpc/platforms/powermac/Makefile b/arch/powerpc/platforms/powermac/Makefile
index f2839eed0f89..561a67d65e4d 100644
--- a/arch/powerpc/platforms/powermac/Makefile
+++ b/arch/powerpc/platforms/powermac/Makefile
@@ -3,7 +3,7 @@ CFLAGS_bootx_init.o  		+= -fPIC
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not trace early boot code
-CFLAGS_REMOVE_bootx_init.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_bootx_init.o = $(CC_FLAGS_FTRACE)
 endif
 
 obj-y				+= pic.o setup.o time.o feature.o pci.o \
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index 1bc3abb237cd..93cc1f1b8b61 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -8,7 +8,7 @@ UBSAN_SANITIZE := n
 
 # Disable ftrace for the entire directory
 ORIG_CFLAGS := $(KBUILD_CFLAGS)
-KBUILD_CFLAGS = $(subst -mno-sched-epilog,,$(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS)))
+KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
 
 ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog
  2018-09-14  4:06 ` [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog Joel Stanley
@ 2018-09-14  5:06   ` Nicholas Piggin
  2018-09-14 18:03     ` Nick Desaulniers
  0 siblings, 1 reply; 26+ messages in thread
From: Nicholas Piggin @ 2018-09-14  5:06 UTC (permalink / raw)
  To: Joel Stanley; +Cc: linuxppc-dev, Nick Desaulniers

On Fri, 14 Sep 2018 13:36:49 +0930
Joel Stanley <joel@jms.id.au> wrote:

> This effectively reverts 7563dc645853 ("powerpc: Work around gcc's
> -fno-omit-frame-pointer bug"), a workaround for a bug in GCC 4.1.3 when
> building 2.6.26 kernel.
> 
> The flag is not supported by clang, but reading the history of the
> bug[1] suggests it is no longer required by supported GCC versions, with
> GCC 4.6 now being the minimum.
> 
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=11414
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> Last time this was proposed there was an issue reported:
> 
>  https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-September/121214.html
> 
> If some machines still have an issue, we could instead wrap this in a
> flag check.

I don't think we can remove it completely because up to at least 4.6
maybe 4.8 has problems.

I have a few patches lying around I started looking at this... I'll
send them.

Thanks,
Nick

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/5] powerpc/Makefiles: Fix clang/llvm build
  2018-09-14  4:06 ` [PATCH v2 1/5] powerpc/Makefiles: Fix clang/llvm build Joel Stanley
@ 2018-09-14 17:41   ` Nick Desaulniers
  2018-09-20  7:04     ` Joel Stanley
  0 siblings, 1 reply; 26+ messages in thread
From: Nick Desaulniers @ 2018-09-14 17:41 UTC (permalink / raw)
  To: joel; +Cc: linuxppc-dev, anton, anton

On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
>
> From: Anton Blanchard <anton@samba.org>
>
> Commit 15a3204d24a3 ("powerpc/64s: Set assembler machine type to POWER4")
> passes -mpower4 to the assembler. We have more recent instructions in our
> assembly files, but gas permits them. The clang/llvm integrated assembler
> is more strict, and we get a build failure.

Note that we disable clang's integrated assembler in the top level
Makefile for now, but it will still validate constraints for inline
assembly.  Do you know which case is meant by "build failure?"

Is there a link to the Clang bug?  It would be good to have that
context in the commit message.

>
> Fix this by calling the assembler with -mcpu=power8 if as supports it,
> else fall back to power4.
>
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  arch/powerpc/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 11a1acba164a..a70639482053 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -238,7 +238,7 @@ cpu-as-$(CONFIG_4xx)                += -Wa,-m405
>  cpu-as-$(CONFIG_ALTIVEC)       += $(call as-option,-Wa$(comma)-maltivec)
>  cpu-as-$(CONFIG_E200)          += -Wa,-me200
>  cpu-as-$(CONFIG_E500)          += -Wa,-me500
> -cpu-as-$(CONFIG_PPC_BOOK3S_64) += -Wa,-mpower4
> +cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call as-option,-Wa$(comma)-mpower8,-Wa$(comma)-mpower4)
>  cpu-as-$(CONFIG_PPC_E500MC)    += $(call as-option,-Wa$(comma)-me500mc)
>
>  KBUILD_AFLAGS += $(cpu-as-y)
> --
> 2.17.1
>

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang
  2018-09-14  4:06 ` [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang Joel Stanley
@ 2018-09-14 17:47   ` Nick Desaulniers
  2018-09-14 21:08     ` Segher Boessenkool
  0 siblings, 1 reply; 26+ messages in thread
From: Nick Desaulniers @ 2018-09-14 17:47 UTC (permalink / raw)
  To: joel; +Cc: linuxppc-dev, anton

On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
>
> Clang's assembler does not like the syntax of the cmpdi:
>
>  arch/powerpc/boot/crt0.S:168:22: error: unexpected modifier on variable reference
>          cmpdi   12,RELACOUNT@l
>                               ^
>  arch/powerpc/boot/crt0.S:168:11: error: unknown operand
>          cmpdi   12,RELACOUNT@l
>                    ^
> Enclosing RELACOUNT in () makes it is happy. Tested with GCC 8 and Clang
> 8 (trunk).
>
> Reported to clang as https://bugs.llvm.org/show_bug.cgi?id=38945
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> v2: Fix for !powerpc64 too, add bug link to commit message
> ---
>  arch/powerpc/boot/crt0.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
> index dcf2f15e6797..e453e011d7e7 100644
> --- a/arch/powerpc/boot/crt0.S
> +++ b/arch/powerpc/boot/crt0.S
> @@ -80,7 +80,7 @@ p_base:       mflr    r10             /* r10 now points to runtime addr of p_base */
>         lwz     r9,4(r12)       /* get RELA pointer in r9 */
>         b       12f
>  11:    addis   r8,r8,(-RELACOUNT)@ha
> -       cmpwi   r8,RELACOUNT@l
> +       cmpwi   r8,(RELACOUNT)@l
>         bne     12f
>         lwz     r0,4(r12)       /* get RELACOUNT value in r0 */
>  12:    addi    r12,r12,8
> @@ -165,7 +165,7 @@ p_base:     mflr    r10             /* r10 now points to runtime addr of p_base */
>         ld      r13,8(r11)       /* get RELA pointer in r13 */
>         b       11f
>  10:    addis   r12,r12,(-RELACOUNT)@ha
> -       cmpdi   r12,RELACOUNT@l
> +       cmpdi   r12,(RELACOUNT)@l

Yep, as we can see above, when RELACOUNT is negated, it's wrapped in
parens.  It's important for Clang's assembler to match GAS eventually,
but for now, this change simply cononicalizes all of the the
references to RELACOUNT in this source file.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>         bne     11f
>         ld      r8,8(r11)       /* get RELACOUNT value in r8 */
>  11:    addi    r11,r11,16
> --
> 2.17.1
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/5] powerpc/boot: Ensure _zimage_start is a weak symbol
  2018-09-14  4:06 ` [PATCH v2 3/5] powerpc/boot: Ensure _zimage_start is a weak symbol Joel Stanley
@ 2018-09-14 17:50   ` Nick Desaulniers
  2018-09-20  4:21   ` [v2,3/5] " Michael Ellerman
  1 sibling, 0 replies; 26+ messages in thread
From: Nick Desaulniers @ 2018-09-14 17:50 UTC (permalink / raw)
  To: joel; +Cc: linuxppc-dev, anton

On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
>
> When building with clang crt0's _zimage_start is not marked weak, which
> breaks the build when linking the kernel image:
>
>  $ objdump -t arch/powerpc/boot/crt0.o |grep _zimage_start$
>  0000000000000058 g       .text  0000000000000000 _zimage_start
>
>  ld: arch/powerpc/boot/wrapper.a(crt0.o): in function '_zimage_start':
>  (.text+0x58): multiple definition of '_zimage_start';
>  arch/powerpc/boot/pseries-head.o:(.text+0x0): first defined here
>
> Clang requires the .weak directive to appear after the symbol is
> declared. The binutils manual says:
>
>  This directive sets the weak attribute on the comma separated list of
>  symbol names. If the symbols do not already exist, they will be
>  created.
>
> So it appears this is different with clang. The only reference I could
> see for this was an OpenBSD mailing list post[1].
>
> Changing it to be after the declaration fixes building with Clang, and
> still works with GCC.
>
>  $ objdump -t arch/powerpc/boot/crt0.o |grep _zimage_start$
>  0000000000000058  w      .text 0000000000000000 _zimage_start
>
> Reported to clang as https://bugs.llvm.org/show_bug.cgi?id=38921
>
> [1] https://groups.google.com/forum/#!topic/fa.openbsd.tech/PAgKKen2YCY
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> v2: Add comment explaining this is a workaround for clang. I have also
> opened a clang bug to see if they plan on changing behaviour
> ---
>  arch/powerpc/boot/crt0.S | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
> index e453e011d7e7..4e32bd251b99 100644
> --- a/arch/powerpc/boot/crt0.S
> +++ b/arch/powerpc/boot/crt0.S
> @@ -47,8 +47,10 @@ p_end:               .long   _end
>  p_pstack:      .long   _platform_stack_top
>  #endif
>
> -       .weak   _zimage_start
>         .globl  _zimage_start
> +       /* Clang appears to require the .weak directive to be after the symbol
> +        * is defined. See https://bugs.llvm.org/show_bug.cgi?id=38921  */
> +       .weak   _zimage_start
>  _zimage_start:
>         .globl  _zimage_start_lib
>  _zimage_start_lib:
> --
> 2.17.1
>

Sure, definitely a long tail of binutils/GAS functionality to match
from Clang's integrated assembler, but this is a small fix that
results in no functional change for GCC/binutils and enables
Clang/bfd.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code
  2018-09-14  4:06 ` [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code Joel Stanley
@ 2018-09-14 17:57   ` Nick Desaulniers
  2018-09-19  7:45     ` Joel Stanley
  2018-09-20  4:21   ` [v2, " Michael Ellerman
  1 sibling, 1 reply; 26+ messages in thread
From: Nick Desaulniers @ 2018-09-14 17:57 UTC (permalink / raw)
  To: joel; +Cc: linuxppc-dev, anton, anton

On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
>
> From: Anton Blanchard <anton@samba.org>
>
> This re-applies b91c1e3e7a6f which was reverted in f2ca80905929
> d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer
> ...").
>
> We see a large number of duplicate const errors in the user access
> code when building with llvm/clang:
>
>   include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier
>       [-Wduplicate-decl-specifier]
>         ret = __get_user(c, uaddr);
>
> The problem is we are doing const __typeof__(*(ptr)), which will hit the
> warning if ptr is marked const.
>
> Removing const does not seem to have any effect on GCC code generation.

I wouldn't expect it to for a local variable with such localized
usage.  I myself am quite liberal in applying `const` to everything,
so I will try to fix this in Clang as well, but this should silence
the warning for users of older versions of Clang and results in no
functional change.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> If we don't want to apply this, other options are suppressing the
> warning, or wait for a fix to land in clang
> (https://github.com/ClangBuiltLinux/linux/issues/52).
> ---
>  arch/powerpc/include/asm/uaccess.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index bac225bb7f64..15bea9a0f260 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -260,7 +260,7 @@ do {                                                                \
>  ({                                                             \
>         long __gu_err;                                          \
>         __long_type(*(ptr)) __gu_val;                           \
> -       const __typeof__(*(ptr)) __user *__gu_addr = (ptr);     \
> +       __typeof__(*(ptr)) __user *__gu_addr = (ptr);   \
>         __chk_user_ptr(ptr);                                    \
>         if (!is_kernel_addr((unsigned long)__gu_addr))          \
>                 might_fault();                                  \
> @@ -274,7 +274,7 @@ do {                                                                \
>  ({                                                                     \
>         long __gu_err = -EFAULT;                                        \
>         __long_type(*(ptr)) __gu_val = 0;                               \
> -       const __typeof__(*(ptr)) __user *__gu_addr = (ptr);             \
> +       __typeof__(*(ptr)) __user *__gu_addr = (ptr);           \
>         might_fault();                                                  \
>         if (access_ok(VERIFY_READ, __gu_addr, (size))) {                \
>                 barrier_nospec();                                       \
> @@ -288,7 +288,7 @@ do {                                                                \
>  ({                                                             \
>         long __gu_err;                                          \
>         __long_type(*(ptr)) __gu_val;                           \
> -       const __typeof__(*(ptr)) __user *__gu_addr = (ptr);     \
> +       __typeof__(*(ptr)) __user *__gu_addr = (ptr);   \
>         __chk_user_ptr(ptr);                                    \
>         barrier_nospec();                                       \
>         __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
> --
> 2.17.1
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog
  2018-09-14  5:06   ` Nicholas Piggin
@ 2018-09-14 18:03     ` Nick Desaulniers
  2018-09-14 20:43       ` Nicholas Piggin
  0 siblings, 1 reply; 26+ messages in thread
From: Nick Desaulniers @ 2018-09-14 18:03 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: joel, linuxppc-dev

On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
> Last time this was proposed there was an issue reported:
>
>  https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-September/121214.html

Heh, did PASemi sell boxes? Interesting, I'll have to read up on my history.

On Thu, Sep 13, 2018 at 10:06 PM Nicholas Piggin <npiggin@gmail.com> wrote:
> I don't think we can remove it completely because up to at least 4.6
> maybe 4.8 has problems.
>
> I have a few patches lying around I started looking at this... I'll
> send them.

Yeah, it's too bad the link above doesn't mention gcc version.

The gcc bugreport mentions fixing the bug in
7563dc64585324f443f5ac107eb6d89ee813a2d2, not sure how to check what
release version of gcc that is? (Do they tag releases?)

Nick, do you have a test case or more context about this still being
an issue in gcc 4.8? (maybe I should wait for your patch series?)

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog
  2018-09-14 18:03     ` Nick Desaulniers
@ 2018-09-14 20:43       ` Nicholas Piggin
  2018-09-14 21:18         ` Segher Boessenkool
  0 siblings, 1 reply; 26+ messages in thread
From: Nicholas Piggin @ 2018-09-14 20:43 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: joel, linuxppc-dev, Segher Boessenkool

On Fri, 14 Sep 2018 11:03:38 -0700
Nick Desaulniers <ndesaulniers@google.com> wrote:

> On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
> > Last time this was proposed there was an issue reported:
> >
> >  https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-September/121214.html  
> 
> Heh, did PASemi sell boxes? Interesting, I'll have to read up on my history.
> 
> On Thu, Sep 13, 2018 at 10:06 PM Nicholas Piggin <npiggin@gmail.com> wrote:
> > I don't think we can remove it completely because up to at least 4.6
> > maybe 4.8 has problems.
> >
> > I have a few patches lying around I started looking at this... I'll
> > send them.  
> 
> Yeah, it's too bad the link above doesn't mention gcc version.
> 
> The gcc bugreport mentions fixing the bug in
> 7563dc64585324f443f5ac107eb6d89ee813a2d2, not sure how to check what
> release version of gcc that is? (Do they tag releases?)

I'm not sure, that's not in my gcc tree AFAIKS.

> 
> Nick, do you have a test case or more context about this still being
> an issue in gcc 4.8? (maybe I should wait for your patch series?)

Sorry I forgot to cc you. This has links to some of the sched
epilog bugs.

https://marc.info/?l=linuxppc-embedded&m=153690223909654&w=2

Thanks,
Nick

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang
  2018-09-14 17:47   ` Nick Desaulniers
@ 2018-09-14 21:08     ` Segher Boessenkool
  2018-09-17 20:41       ` Nick Desaulniers
  0 siblings, 1 reply; 26+ messages in thread
From: Segher Boessenkool @ 2018-09-14 21:08 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: joel, linuxppc-dev

On Fri, Sep 14, 2018 at 10:47:08AM -0700, Nick Desaulniers wrote:
> On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
> >  10:    addis   r12,r12,(-RELACOUNT)@ha
> > -       cmpdi   r12,RELACOUNT@l
> > +       cmpdi   r12,(RELACOUNT)@l
> 
> Yep, as we can see above, when RELACOUNT is negated, it's wrapped in
> parens.

The only thing that does is make it easier for humans to read; it means
exactly the same thing.


Segher

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog
  2018-09-14 20:43       ` Nicholas Piggin
@ 2018-09-14 21:18         ` Segher Boessenkool
  2018-09-14 22:20           ` Nick Desaulniers
  0 siblings, 1 reply; 26+ messages in thread
From: Segher Boessenkool @ 2018-09-14 21:18 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: Nick Desaulniers, joel, linuxppc-dev

On Sat, Sep 15, 2018 at 06:43:05AM +1000, Nicholas Piggin wrote:
> On Fri, 14 Sep 2018 11:03:38 -0700
> Nick Desaulniers <ndesaulniers@google.com> wrote:
> 
> > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
> > > Last time this was proposed there was an issue reported:
> > >
> > >  https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-September/121214.html  
> > 
> > Heh, did PASemi sell boxes? Interesting, I'll have to read up on my history.
> > 
> > On Thu, Sep 13, 2018 at 10:06 PM Nicholas Piggin <npiggin@gmail.com> wrote:
> > > I don't think we can remove it completely because up to at least 4.6
> > > maybe 4.8 has problems.
> > >
> > > I have a few patches lying around I started looking at this... I'll
> > > send them.  
> > 
> > Yeah, it's too bad the link above doesn't mention gcc version.
> > 
> > The gcc bugreport mentions fixing the bug in
> > 7563dc64585324f443f5ac107eb6d89ee813a2d2, not sure how to check what
> > release version of gcc that is? (Do they tag releases?)
> 
> I'm not sure, that's not in my gcc tree AFAIKS.

This is a git hash in the kernel tree.

> > Nick, do you have a test case or more context about this still being
> > an issue in gcc 4.8? (maybe I should wait for your patch series?)
> 
> Sorry I forgot to cc you. This has links to some of the sched
> epilog bugs.
> 
> https://marc.info/?l=linuxppc-embedded&m=153690223909654&w=2

PR44199 was backported to 4.4 and PR52828 is fixed in 4.8.


Segher

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog
  2018-09-14 21:18         ` Segher Boessenkool
@ 2018-09-14 22:20           ` Nick Desaulniers
  2018-09-15  1:04             ` Segher Boessenkool
  0 siblings, 1 reply; 26+ messages in thread
From: Nick Desaulniers @ 2018-09-14 22:20 UTC (permalink / raw)
  To: segher; +Cc: Nicholas Piggin, joel, linuxppc-dev

On Fri, Sep 14, 2018 at 2:56 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Sat, Sep 15, 2018 at 06:43:05AM +1000, Nicholas Piggin wrote:
> > On Fri, 14 Sep 2018 11:03:38 -0700
> > Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
> > > > Last time this was proposed there was an issue reported:
> > > >
> > > >  https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-September/121214.html
> > >
> > > Heh, did PASemi sell boxes? Interesting, I'll have to read up on my history.
> > >
> > > On Thu, Sep 13, 2018 at 10:06 PM Nicholas Piggin <npiggin@gmail.com> wrote:
> > > > I don't think we can remove it completely because up to at least 4.6
> > > > maybe 4.8 has problems.
> > > >
> > > > I have a few patches lying around I started looking at this... I'll
> > > > send them.
> > >
> > > Yeah, it's too bad the link above doesn't mention gcc version.
> > >
> > > The gcc bugreport mentions fixing the bug in
> > > 7563dc64585324f443f5ac107eb6d89ee813a2d2, not sure how to check what
> > > release version of gcc that is? (Do they tag releases?)
> >
> > I'm not sure, that's not in my gcc tree AFAIKS.
>
> This is a git hash in the kernel tree.
>
> > > Nick, do you have a test case or more context about this still being
> > > an issue in gcc 4.8? (maybe I should wait for your patch series?)
> >
> > Sorry I forgot to cc you. This has links to some of the sched
> > epilog bugs.
> >
> > https://marc.info/?l=linuxppc-embedded&m=153690223909654&w=2

Cool, cc me on the thread if you'd like me to add my reviewed-by tag
visibly on the thread.

>
> PR44199 was backported to 4.4 and PR52828 is fixed in 4.8.

Are those GCC versions? If so, what does that mean for the users of
the many GCC releases between 4.4 and 4.8?

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog
  2018-09-14 22:20           ` Nick Desaulniers
@ 2018-09-15  1:04             ` Segher Boessenkool
  0 siblings, 0 replies; 26+ messages in thread
From: Segher Boessenkool @ 2018-09-15  1:04 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: Nicholas Piggin, joel, linuxppc-dev

On Fri, Sep 14, 2018 at 03:20:18PM -0700, Nick Desaulniers wrote:
> On Fri, Sep 14, 2018 at 2:56 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Sat, Sep 15, 2018 at 06:43:05AM +1000, Nicholas Piggin wrote:
> > > On Fri, 14 Sep 2018 11:03:38 -0700
> > > Nick Desaulniers <ndesaulniers@google.com> wrote:
> > >
> > > Sorry I forgot to cc you. This has links to some of the sched
> > > epilog bugs.
> > >
> > > https://marc.info/?l=linuxppc-embedded&m=153690223909654&w=2
> 
> Cool, cc me on the thread if you'd like me to add my reviewed-by tag
> visibly on the thread.
> 
> > PR44199 was backported to 4.4 and PR52828 is fixed in 4.8.
> 
> Are those GCC versions? If so, what does that mean for the users of
> the many GCC releases between 4.4 and 4.8?

Yes, GCC bug 44199 was fixed in GCC version 4.4, etc.  It of course also
is fixed in all later versions; so GCC releases between 4.4 and 4.8 have
the fix for PR44199 but not that for PR52828.

I didn't check exactly what 4.4.x versions have the fix, etc.  I always
assume anyone using x.y.z uses the highest z available.

I don't know if those are the only fixes you need; those are the two
bugs mentioned in Nicholas' patch (that MARC link above).


Segher

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang
  2018-09-14 21:08     ` Segher Boessenkool
@ 2018-09-17 20:41       ` Nick Desaulniers
  2018-09-18  1:08         ` Joel Stanley
  0 siblings, 1 reply; 26+ messages in thread
From: Nick Desaulniers @ 2018-09-17 20:41 UTC (permalink / raw)
  To: segher; +Cc: joel, linuxppc-dev

On Fri, Sep 14, 2018 at 2:08 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Fri, Sep 14, 2018 at 10:47:08AM -0700, Nick Desaulniers wrote:
> > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
> > >  10:    addis   r12,r12,(-RELACOUNT)@ha
> > > -       cmpdi   r12,RELACOUNT@l
> > > +       cmpdi   r12,(RELACOUNT)@l
> >
> > Yep, as we can see above, when RELACOUNT is negated, it's wrapped in
> > parens.

Looks like this was just fixed in Clang-8:
https://bugs.llvm.org/show_bug.cgi?id=38945
https://reviews.llvm.org/D52188

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang
  2018-09-17 20:41       ` Nick Desaulniers
@ 2018-09-18  1:08         ` Joel Stanley
  2018-09-18 10:53           ` Michael Ellerman
  0 siblings, 1 reply; 26+ messages in thread
From: Joel Stanley @ 2018-09-18  1:08 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: Segher Boessenkool, linuxppc-dev

On Tue, 18 Sep 2018 at 06:11, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Fri, Sep 14, 2018 at 2:08 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > On Fri, Sep 14, 2018 at 10:47:08AM -0700, Nick Desaulniers wrote:
> > > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
> > > >  10:    addis   r12,r12,(-RELACOUNT)@ha
> > > > -       cmpdi   r12,RELACOUNT@l
> > > > +       cmpdi   r12,(RELACOUNT)@l
> > >
> > > Yep, as we can see above, when RELACOUNT is negated, it's wrapped in
> > > parens.
>
> Looks like this was just fixed in Clang-8:
> https://bugs.llvm.org/show_bug.cgi?id=38945
> https://reviews.llvm.org/D52188

Nice!

mpe, given we need the local references to labels fix which is also in
clang-8 I suggest we drop this patch.

Cheers,

Joel

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang
  2018-09-18  1:08         ` Joel Stanley
@ 2018-09-18 10:53           ` Michael Ellerman
  0 siblings, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2018-09-18 10:53 UTC (permalink / raw)
  To: Joel Stanley, Nick Desaulniers; +Cc: linuxppc-dev

Joel Stanley <joel@jms.id.au> writes:

> On Tue, 18 Sep 2018 at 06:11, Nick Desaulniers <ndesaulniers@google.com> wrote:
>>
>> On Fri, Sep 14, 2018 at 2:08 PM Segher Boessenkool
>> <segher@kernel.crashing.org> wrote:
>> >
>> > On Fri, Sep 14, 2018 at 10:47:08AM -0700, Nick Desaulniers wrote:
>> > > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
>> > > >  10:    addis   r12,r12,(-RELACOUNT)@ha
>> > > > -       cmpdi   r12,RELACOUNT@l
>> > > > +       cmpdi   r12,(RELACOUNT)@l
>> > >
>> > > Yep, as we can see above, when RELACOUNT is negated, it's wrapped in
>> > > parens.
>>
>> Looks like this was just fixed in Clang-8:
>> https://bugs.llvm.org/show_bug.cgi?id=38945
>> https://reviews.llvm.org/D52188
>
> Nice!
>
> mpe, given we need the local references to labels fix which is also in
> clang-8 I suggest we drop this patch.

OK, no worries.

cheers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code
  2018-09-14 17:57   ` Nick Desaulniers
@ 2018-09-19  7:45     ` Joel Stanley
  2018-09-20  4:54       ` Christophe LEROY
  0 siblings, 1 reply; 26+ messages in thread
From: Joel Stanley @ 2018-09-19  7:45 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: linuxppc-dev, Anton Blanchard, Anton Blanchard

On Sat, 15 Sep 2018 at 03:27, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
> >
> > From: Anton Blanchard <anton@samba.org>
> >
> > This re-applies b91c1e3e7a6f which was reverted in f2ca80905929
> > d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer
> > ...").
> >
> > We see a large number of duplicate const errors in the user access
> > code when building with llvm/clang:
> >
> >   include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier
> >       [-Wduplicate-decl-specifier]
> >         ret = __get_user(c, uaddr);
> >
> > The problem is we are doing const __typeof__(*(ptr)), which will hit the
> > warning if ptr is marked const.
> >
> > Removing const does not seem to have any effect on GCC code generation.
>
> I wouldn't expect it to for a local variable with such localized
> usage.  I myself am quite liberal in applying `const` to everything,
> so I will try to fix this in Clang as well, but this should silence
> the warning for users of older versions of Clang and results in no
> functional change.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Nick has written a clang patch that suppresses the warning in the same
way as GCC. Assuming it gets merged, as we depend on clang-8 we could
chose to not merge the kernel patch.

https://reviews.llvm.org/D52248

Cheers,

Joel

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [v2,3/5] powerpc/boot: Ensure _zimage_start is a weak symbol
  2018-09-14  4:06 ` [PATCH v2 3/5] powerpc/boot: Ensure _zimage_start is a weak symbol Joel Stanley
  2018-09-14 17:50   ` Nick Desaulniers
@ 2018-09-20  4:21   ` Michael Ellerman
  1 sibling, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2018-09-20  4:21 UTC (permalink / raw)
  To: Joel Stanley, linuxppc-dev; +Cc: Nick Desaulniers

On Fri, 2018-09-14 at 04:06:47 UTC, Joel Stanley wrote:
> When building with clang crt0's _zimage_start is not marked weak, which
> breaks the build when linking the kernel image:
> 
>  $ objdump -t arch/powerpc/boot/crt0.o |grep _zimage_start$
>  0000000000000058 g       .text  0000000000000000 _zimage_start
> 
>  ld: arch/powerpc/boot/wrapper.a(crt0.o): in function '_zimage_start':
>  (.text+0x58): multiple definition of '_zimage_start';
>  arch/powerpc/boot/pseries-head.o:(.text+0x0): first defined here
> 
> Clang requires the .weak directive to appear after the symbol is
> declared. The binutils manual says:
> 
>  This directive sets the weak attribute on the comma separated list of
>  symbol names. If the symbols do not already exist, they will be
>  created.
> 
> So it appears this is different with clang. The only reference I could
> see for this was an OpenBSD mailing list post[1].
> 
> Changing it to be after the declaration fixes building with Clang, and
> still works with GCC.
> 
>  $ objdump -t arch/powerpc/boot/crt0.o |grep _zimage_start$
>  0000000000000058  w      .text	0000000000000000 _zimage_start
> 
> Reported to clang as https://bugs.llvm.org/show_bug.cgi?id=38921
> 
> [1] https://groups.google.com/forum/#!topic/fa.openbsd.tech/PAgKKen2YCY
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ee9d21b3b3583712029a0db65a4b7c

cheers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [v2, 4/5] powerpc: Fix duplicate const clang warning in user access code
  2018-09-14  4:06 ` [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code Joel Stanley
  2018-09-14 17:57   ` Nick Desaulniers
@ 2018-09-20  4:21   ` Michael Ellerman
  1 sibling, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2018-09-20  4:21 UTC (permalink / raw)
  To: Joel Stanley, linuxppc-dev; +Cc: Nick Desaulniers, Anton Blanchard

On Fri, 2018-09-14 at 04:06:48 UTC, Joel Stanley wrote:
> From: Anton Blanchard <anton@samba.org>
> 
> This re-applies b91c1e3e7a6f which was reverted in f2ca80905929
> d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer
> ...").
> 
> We see a large number of duplicate const errors in the user access
> code when building with llvm/clang:
> 
>   include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier
>       [-Wduplicate-decl-specifier]
>         ret = __get_user(c, uaddr);
> 
> The problem is we are doing const __typeof__(*(ptr)), which will hit the
> warning if ptr is marked const.
> 
> Removing const does not seem to have any effect on GCC code generation.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/e00d93ac9a189673028ac125a74b9b

cheers

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code
  2018-09-19  7:45     ` Joel Stanley
@ 2018-09-20  4:54       ` Christophe LEROY
  2018-09-20 12:35         ` Michael Ellerman
  0 siblings, 1 reply; 26+ messages in thread
From: Christophe LEROY @ 2018-09-20  4:54 UTC (permalink / raw)
  To: Joel Stanley, Nick Desaulniers; +Cc: linuxppc-dev, Anton Blanchard



Le 19/09/2018 à 09:45, Joel Stanley a écrit :
> On Sat, 15 Sep 2018 at 03:27, Nick Desaulniers <ndesaulniers@google.com> wrote:
>>
>> On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
>>>
>>> From: Anton Blanchard <anton@samba.org>
>>>
>>> This re-applies b91c1e3e7a6f which was reverted in f2ca80905929
>>> d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer
>>> ...").
>>>
>>> We see a large number of duplicate const errors in the user access
>>> code when building with llvm/clang:
>>>
>>>    include/linux/pagemap.h:576:8: warning: duplicate 'const' declaration specifier
>>>        [-Wduplicate-decl-specifier]
>>>          ret = __get_user(c, uaddr);
>>>
>>> The problem is we are doing const __typeof__(*(ptr)), which will hit the
>>> warning if ptr is marked const.
>>>
>>> Removing const does not seem to have any effect on GCC code generation.
>>
>> I wouldn't expect it to for a local variable with such localized
>> usage.  I myself am quite liberal in applying `const` to everything,
>> so I will try to fix this in Clang as well, but this should silence
>> the warning for users of older versions of Clang and results in no
>> functional change.
>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> Nick has written a clang patch that suppresses the warning in the same
> way as GCC. Assuming it gets merged, as we depend on clang-8 we could
> chose to not merge the kernel patch.
> 
> https://reviews.llvm.org/D52248
> 

Seems like Michael has merged this patch anyway.

Christophe

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/5] powerpc/Makefiles: Fix clang/llvm build
  2018-09-14 17:41   ` Nick Desaulniers
@ 2018-09-20  7:04     ` Joel Stanley
  0 siblings, 0 replies; 26+ messages in thread
From: Joel Stanley @ 2018-09-20  7:04 UTC (permalink / raw)
  To: Nick Desaulniers, Michael Ellerman, Segher Boessenkool
  Cc: linuxppc-dev, Anton Blanchard, Anton Blanchard

On Sat, 15 Sep 2018 at 03:11, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
> >
> > From: Anton Blanchard <anton@samba.org>
> >
> > Commit 15a3204d24a3 ("powerpc/64s: Set assembler machine type to POWER4")
> > passes -mpower4 to the assembler. We have more recent instructions in our
> > assembly files, but gas permits them. The clang/llvm integrated assembler
> > is more strict, and we get a build failure.
>
> Note that we disable clang's integrated assembler in the top level
> Makefile for now, but it will still validate constraints for inline
> assembly.  Do you know which case is meant by "build failure?"

An example of a failure is:

  CC      init/do_mounts.o
/tmp/do_mounts-58cd1d.s: Assembler messages:
/tmp/do_mounts-58cd1d.s:251: Error: unrecognized opcode: `isel'
/tmp/do_mounts-58cd1d.s:748: Error: unrecognized opcode: `isel'
/tmp/do_mounts-58cd1d.s:759: Error: unrecognized opcode: `isel'
/tmp/do_mounts-58cd1d.s:1373: Error: unrecognized opcode: `isel'

The full build line at the bottom of this email. The m flags:

 -mlittle-endian -m64 -msoft-float -mabi=elfv2 -mcmodel=medium
-mcpu=power8 -mtune=power9 -mno-altivec -mno-vsx -Wa,-maltivec
-Wa,-mpower4 -mlittle-endian

If we remove the -Wa,-mpower4 it works. This is because isel is not
available on power4.

Digging deeper, clang drives gas like this:

"/usr/bin/as" -a64 -mppc64 -mlittle-endian -mpower8 -I
./arch/powerpc/include -I ./arch/powerpc/include/generated -I
./include -I ./arch/powerpc/include/uapi -I
./arch/powerpc/include/generated/uapi -I ./include/uapi -I
./include/generated/uapi -I arch/powerpc -I arch/powerpc -maltivec
-mpower4 -o init/do_mounts.o /tmp/do_mounts-3b0a3d.s

GCC drives it:

as -v -I ./arch/powerpc/include -I ./arch/powerpc/include/generated -I
./include -I ./arch/powerpc/include/uapi -I
./arch/powerpc/include/generated/uapi -I ./include/uapi -I
./include/generated/uapi -I arch/powerpc -I arch/powerpc -a64 -mpower8
-many -mlittle -maltivec -mpower4 -o init/do_mounts.o

I think the important difference is GCC adds -many:

        -many
           Generate code for any architecture (PWR/PWRX/PPC).

This is probably what we mean when building the kernel. I will get
some advice from our toolchain people first to understand what -many
does.

> Is there a link to the Clang bug?  It would be good to have that
> context in the commit message.

I haven't opened a clang bug. I'm not sure that we need one, unless we
want it to add -many to the assembler call like gcc does.

Cheers,

Joel

--
  /scratch/joel/llvm-build/bin/clang-8 -Wp,-MD,init/.do_mounts.o.d
-nostdinc -isystem /scratch/joel/llvm-build/lib/clang/8.0.0/include
-I./arch/powerpc/include -I./arch/powerpc/include/generated
-I./include -I./arch/powerpc/include/uapi
-I./arch/powerpc/include/generated/uapi -I./include/uapi
-I./include/generated/uapi -include ./include/linux/kconfig.h -include
./include/linux/compiler_types.h -D__KERNEL__ -Iarch/powerpc
-DHAVE_AS_ATHIGH=1 -Qunused-arguments -Wall -Wundef
-Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common
-fshort-wchar -Werror-implicit-function-declaration
-Wno-format-security -std=gnu89 -no-integrated-as -fno-PIE
-mlittle-endian -m64 -msoft-float -pipe -Iarch/powerpc -mabi=elfv2
-mcmodel=medium -mcpu=power8 -mtune=power9 -mno-altivec -mno-vsx
-funit-at-a-time -fno-dwarf2-cfi-asm -Wa,-maltivec -Wa,-mpower4
-mlittle-endian -fno-delete-null-pointer-checks -Oz
-Wframe-larger-than=2048 -fno-stack-protector
-Wno-format-invalid-specifier -Wno-gnu -Wno-address-of-packed-member
-Wno-tautological-compare -mno-global-merge -Wno-unused-const-variable
-fomit-frame-pointer -Wdeclaration-after-statement -Wno-pointer-sign
-fno-strict-overflow -fno-merge-all-constants -fno-stack-check
-Werror=implicit-int -Werror=strict-prototypes -Werror=date-time
-Werror=incompatible-pointer-types -Wno-initializer-overrides
-Wno-unused-value -Wno-format -Wno-sign-compare
-Wno-format-zero-length -Wno-uninitialized -fno-function-sections
-fno-data-sections    -DKBUILD_BASENAME='"do_mounts"'
-DKBUILD_MODNAME='"mounts"' -c -o init/do_mounts.o init/do_mounts.c

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code
  2018-09-20  4:54       ` Christophe LEROY
@ 2018-09-20 12:35         ` Michael Ellerman
  0 siblings, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2018-09-20 12:35 UTC (permalink / raw)
  To: Christophe LEROY, Joel Stanley, Nick Desaulniers
  Cc: linuxppc-dev, Anton Blanchard

Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 19/09/2018 =C3=A0 09:45, Joel Stanley a =C3=A9crit=C2=A0:
>> On Sat, 15 Sep 2018 at 03:27, Nick Desaulniers <ndesaulniers@google.com>=
 wrote:
>>>
>>> On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
>>>>
>>>> From: Anton Blanchard <anton@samba.org>
>>>>
>>>> This re-applies b91c1e3e7a6f which was reverted in f2ca80905929
>>>> d466f6c5cac1 f84ed59a612d (powerpc/sparse: Constify the address pointer
>>>> ...").
>>>>
>>>> We see a large number of duplicate const errors in the user access
>>>> code when building with llvm/clang:
>>>>
>>>>    include/linux/pagemap.h:576:8: warning: duplicate 'const' declarati=
on specifier
>>>>        [-Wduplicate-decl-specifier]
>>>>          ret =3D __get_user(c, uaddr);
>>>>
>>>> The problem is we are doing const __typeof__(*(ptr)), which will hit t=
he
>>>> warning if ptr is marked const.
>>>>
>>>> Removing const does not seem to have any effect on GCC code generation.
>>>
>>> I wouldn't expect it to for a local variable with such localized
>>> usage.  I myself am quite liberal in applying `const` to everything,
>>> so I will try to fix this in Clang as well, but this should silence
>>> the warning for users of older versions of Clang and results in no
>>> functional change.
>>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>>=20
>> Nick has written a clang patch that suppresses the warning in the same
>> way as GCC. Assuming it gets merged, as we depend on clang-8 we could
>> chose to not merge the kernel patch.
>>=20
>> https://reviews.llvm.org/D52248
>
> Seems like Michael has merged this patch anyway.

I'll happily revert it though if someone tells me to.

cheers

^ permalink raw reply	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2018-09-20 12:35 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-14  4:06 [PATCH v2 0/5] powerpc: Clang build fixes Joel Stanley
2018-09-14  4:06 ` [PATCH v2 1/5] powerpc/Makefiles: Fix clang/llvm build Joel Stanley
2018-09-14 17:41   ` Nick Desaulniers
2018-09-20  7:04     ` Joel Stanley
2018-09-14  4:06 ` [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang Joel Stanley
2018-09-14 17:47   ` Nick Desaulniers
2018-09-14 21:08     ` Segher Boessenkool
2018-09-17 20:41       ` Nick Desaulniers
2018-09-18  1:08         ` Joel Stanley
2018-09-18 10:53           ` Michael Ellerman
2018-09-14  4:06 ` [PATCH v2 3/5] powerpc/boot: Ensure _zimage_start is a weak symbol Joel Stanley
2018-09-14 17:50   ` Nick Desaulniers
2018-09-20  4:21   ` [v2,3/5] " Michael Ellerman
2018-09-14  4:06 ` [PATCH v2 4/5] powerpc: Fix duplicate const clang warning in user access code Joel Stanley
2018-09-14 17:57   ` Nick Desaulniers
2018-09-19  7:45     ` Joel Stanley
2018-09-20  4:54       ` Christophe LEROY
2018-09-20 12:35         ` Michael Ellerman
2018-09-20  4:21   ` [v2, " Michael Ellerman
2018-09-14  4:06 ` [PATCH v2 5/5] powerpc: Remove -mno-sched-epilog Joel Stanley
2018-09-14  5:06   ` Nicholas Piggin
2018-09-14 18:03     ` Nick Desaulniers
2018-09-14 20:43       ` Nicholas Piggin
2018-09-14 21:18         ` Segher Boessenkool
2018-09-14 22:20           ` Nick Desaulniers
2018-09-15  1:04             ` Segher Boessenkool

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.