[PATCH v2] raid6/ppc: Fix build for clang

[PATCH v2] raid6/ppc: Fix build for clang

[Joel Stanley]

We cannot build these files with clang as it does not allow altivec
instructions in assembly when -msoft-float is passed.

Jinsong Ji <jji@us.ibm.com> wrote:
> We currently disable Altivec/VSX support when enabling soft-float.  So
> any usage of vector builtins will break.
>
> Enable Altivec/VSX with soft-float may need quite some clean up work, so
> I guess this is currently a limitation.
>
> Removing -msoft-float will make it work (and we are lucky that no
> floating point instructions will be generated as well).

This is a workaround until the issue is resolved in clang.

Link: https://bugs.llvm.org/show_bug.cgi?id=31177
Link: https://github.com/ClangBuiltLinux/linux/issues/239
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v2: fix typo in comment, thanks Jinsong

 lib/raid6/Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/raid6/Makefile b/lib/raid6/Makefile
index 2f8b61dfd9b0..7ed43eaa02ef 100644
--- a/lib/raid6/Makefile
+++ b/lib/raid6/Makefile
@@ -18,6 +18,21 @@ quiet_cmd_unroll = UNROLL  $@
 
 ifeq ($(CONFIG_ALTIVEC),y)
 altivec_flags := -maltivec $(call cc-option,-mabi=altivec)
+
+ifdef CONFIG_CC_IS_CLANG
+# clang ppc port does not yet support -maltivec when -msoft-float is
+# enabled. A future release of clang will resolve this
+# https://bugs.llvm.org/show_bug.cgi?id=31177
+CFLAGS_REMOVE_altivec1.o  += -msoft-float
+CFLAGS_REMOVE_altivec2.o  += -msoft-float
+CFLAGS_REMOVE_altivec4.o  += -msoft-float
+CFLAGS_REMOVE_altivec8.o  += -msoft-float
+CFLAGS_REMOVE_altivec8.o  += -msoft-float
+CFLAGS_REMOVE_vpermxor1.o += -msoft-float
+CFLAGS_REMOVE_vpermxor2.o += -msoft-float
+CFLAGS_REMOVE_vpermxor4.o += -msoft-float
+CFLAGS_REMOVE_vpermxor8.o += -msoft-float
+endif
 endif
 
 # The GCC option -ffreestanding is required in order to compile code containing
-- 
2.19.1

Re: [PATCH v2] raid6/ppc: Fix build for clang

[Nick Desaulniers]

On Thu, Nov 1, 2018 at 5:45 PM Joel Stanley <joel@jms.id.au> wrote:
>
> We cannot build these files with clang as it does not allow altivec
> instructions in assembly when -msoft-float is passed.
>
> Jinsong Ji <jji@us.ibm.com> wrote:
> > We currently disable Altivec/VSX support when enabling soft-float.  So
> > any usage of vector builtins will break.
> >
> > Enable Altivec/VSX with soft-float may need quite some clean up work, so
> > I guess this is currently a limitation.
> >
> > Removing -msoft-float will make it work (and we are lucky that no
> > floating point instructions will be generated as well).
>
> This is a workaround until the issue is resolved in clang.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=31177
> Link: https://github.com/ClangBuiltLinux/linux/issues/239
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> v2: fix typo in comment, thanks Jinsong
>
>  lib/raid6/Makefile | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/lib/raid6/Makefile b/lib/raid6/Makefile
> index 2f8b61dfd9b0..7ed43eaa02ef 100644
> --- a/lib/raid6/Makefile
> +++ b/lib/raid6/Makefile
> @@ -18,6 +18,21 @@ quiet_cmd_unroll = UNROLL  $@
>
>  ifeq ($(CONFIG_ALTIVEC),y)
>  altivec_flags := -maltivec $(call cc-option,-mabi=altivec)
> +
> +ifdef CONFIG_CC_IS_CLANG
> +# clang ppc port does not yet support -maltivec when -msoft-float is
> +# enabled. A future release of clang will resolve this
> +# https://bugs.llvm.org/show_bug.cgi?id=31177
> +CFLAGS_REMOVE_altivec1.o  += -msoft-float
> +CFLAGS_REMOVE_altivec2.o  += -msoft-float
> +CFLAGS_REMOVE_altivec4.o  += -msoft-float
> +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> +CFLAGS_REMOVE_vpermxor1.o += -msoft-float
> +CFLAGS_REMOVE_vpermxor2.o += -msoft-float
> +CFLAGS_REMOVE_vpermxor4.o += -msoft-float
> +CFLAGS_REMOVE_vpermxor8.o += -msoft-float
> +endif

Hi Joel, thanks for this patch!  My same thoughts about
CONFIG_CC_IS_CLANG vs cc-option from
https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-November/180939.html
apply here as well.  I don't feel strongly about either though.  What
are your thoughts?

>  endif
>
>  # The GCC option -ffreestanding is required in order to compile code containing
> --
> 2.19.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2] raid6/ppc: Fix build for clang

[Joel Stanley]

On Sat, 3 Nov 2018 at 04:04, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Thu, Nov 1, 2018 at 5:45 PM Joel Stanley <joel@jms.id.au> wrote:
> >
> > We cannot build these files with clang as it does not allow altivec
> > instructions in assembly when -msoft-float is passed.
> >
> > Jinsong Ji <jji@us.ibm.com> wrote:
> > > We currently disable Altivec/VSX support when enabling soft-float.  So
> > > any usage of vector builtins will break.
> > >
> > > Enable Altivec/VSX with soft-float may need quite some clean up work, so
> > > I guess this is currently a limitation.
> > >
> > > Removing -msoft-float will make it work (and we are lucky that no
> > > floating point instructions will be generated as well).
> >
> > This is a workaround until the issue is resolved in clang.
> >
> > Link: https://bugs.llvm.org/show_bug.cgi?id=31177
> > Link: https://github.com/ClangBuiltLinux/linux/issues/239
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> > v2: fix typo in comment, thanks Jinsong
> >
> >  lib/raid6/Makefile | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/lib/raid6/Makefile b/lib/raid6/Makefile
> > index 2f8b61dfd9b0..7ed43eaa02ef 100644
> > --- a/lib/raid6/Makefile
> > +++ b/lib/raid6/Makefile
> > @@ -18,6 +18,21 @@ quiet_cmd_unroll = UNROLL  $@
> >
> >  ifeq ($(CONFIG_ALTIVEC),y)
> >  altivec_flags := -maltivec $(call cc-option,-mabi=altivec)
> > +
> > +ifdef CONFIG_CC_IS_CLANG
> > +# clang ppc port does not yet support -maltivec when -msoft-float is
> > +# enabled. A future release of clang will resolve this
> > +# https://bugs.llvm.org/show_bug.cgi?id=31177
> > +CFLAGS_REMOVE_altivec1.o  += -msoft-float
> > +CFLAGS_REMOVE_altivec2.o  += -msoft-float
> > +CFLAGS_REMOVE_altivec4.o  += -msoft-float
> > +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> > +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> > +CFLAGS_REMOVE_vpermxor1.o += -msoft-float
> > +CFLAGS_REMOVE_vpermxor2.o += -msoft-float
> > +CFLAGS_REMOVE_vpermxor4.o += -msoft-float
> > +CFLAGS_REMOVE_vpermxor8.o += -msoft-float
> > +endif
>
> Hi Joel, thanks for this patch!  My same thoughts about
> CONFIG_CC_IS_CLANG vs cc-option from
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-November/180939.html
> apply here as well.  I don't feel strongly about either though.  What
> are your thoughts?

I'm not sure that we can test for this one with cc-option. The result
of having -maltivec with -msoft-float is a error about the internals
of clang, which isn't something that kbuild is set up to test for.

When clang is fixed to allow this combination we will still build this
code in the same way, so in that sense it fails "open".

Cheers,

Joel

Re: [PATCH v2] raid6/ppc: Fix build for clang

[Nick Desaulniers]

On Mon, Dec 3, 2018 at 2:24 AM Joel Stanley <joel@jms.id.au> wrote:
>
> On Sat, 3 Nov 2018 at 04:04, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Thu, Nov 1, 2018 at 5:45 PM Joel Stanley <joel@jms.id.au> wrote:
> > >
> > > We cannot build these files with clang as it does not allow altivec
> > > instructions in assembly when -msoft-float is passed.
> > >
> > > Jinsong Ji <jji@us.ibm.com> wrote:
> > > > We currently disable Altivec/VSX support when enabling soft-float.  So
> > > > any usage of vector builtins will break.
> > > >
> > > > Enable Altivec/VSX with soft-float may need quite some clean up work, so
> > > > I guess this is currently a limitation.
> > > >
> > > > Removing -msoft-float will make it work (and we are lucky that no
> > > > floating point instructions will be generated as well).
> > >
> > > This is a workaround until the issue is resolved in clang.
> > >
> > > Link: https://bugs.llvm.org/show_bug.cgi?id=31177
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/239
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > ---
> > > v2: fix typo in comment, thanks Jinsong
> > >
> > >  lib/raid6/Makefile | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/lib/raid6/Makefile b/lib/raid6/Makefile
> > > index 2f8b61dfd9b0..7ed43eaa02ef 100644
> > > --- a/lib/raid6/Makefile
> > > +++ b/lib/raid6/Makefile
> > > @@ -18,6 +18,21 @@ quiet_cmd_unroll = UNROLL  $@
> > >
> > >  ifeq ($(CONFIG_ALTIVEC),y)
> > >  altivec_flags := -maltivec $(call cc-option,-mabi=altivec)
> > > +
> > > +ifdef CONFIG_CC_IS_CLANG
> > > +# clang ppc port does not yet support -maltivec when -msoft-float is
> > > +# enabled. A future release of clang will resolve this
> > > +# https://bugs.llvm.org/show_bug.cgi?id=31177
> > > +CFLAGS_REMOVE_altivec1.o  += -msoft-float
> > > +CFLAGS_REMOVE_altivec2.o  += -msoft-float
> > > +CFLAGS_REMOVE_altivec4.o  += -msoft-float
> > > +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> > > +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> > > +CFLAGS_REMOVE_vpermxor1.o += -msoft-float
> > > +CFLAGS_REMOVE_vpermxor2.o += -msoft-float
> > > +CFLAGS_REMOVE_vpermxor4.o += -msoft-float
> > > +CFLAGS_REMOVE_vpermxor8.o += -msoft-float
> > > +endif
> >
> > Hi Joel, thanks for this patch!  My same thoughts about
> > CONFIG_CC_IS_CLANG vs cc-option from
> > https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-November/180939.html
> > apply here as well.  I don't feel strongly about either though.  What
> > are your thoughts?
>
> I'm not sure that we can test for this one with cc-option. The result
> of having -maltivec with -msoft-float is a error about the internals
> of clang, which isn't something that kbuild is set up to test for.

As in clang itself crashes, and cc-option/kbuild can't handle that gracefully?

>
> When clang is fixed to allow this combination we will still build this
> code in the same way, so in that sense it fails "open".
>
> Cheers,
>
> Joel



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2] raid6/ppc: Fix build for clang

[Joel Stanley]

On Tue, 4 Dec 2018 at 05:15, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > > +ifdef CONFIG_CC_IS_CLANG
> > > > +# clang ppc port does not yet support -maltivec when -msoft-float is
> > > > +# enabled. A future release of clang will resolve this
> > > > +# https://bugs.llvm.org/show_bug.cgi?id=31177
> > > > +CFLAGS_REMOVE_altivec1.o  += -msoft-float
> > > > +CFLAGS_REMOVE_altivec2.o  += -msoft-float
> > > > +CFLAGS_REMOVE_altivec4.o  += -msoft-float
> > > > +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> > > > +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> > > > +CFLAGS_REMOVE_vpermxor1.o += -msoft-float
> > > > +CFLAGS_REMOVE_vpermxor2.o += -msoft-float
> > > > +CFLAGS_REMOVE_vpermxor4.o += -msoft-float
> > > > +CFLAGS_REMOVE_vpermxor8.o += -msoft-float
> > > > +endif
> > >
> > > Hi Joel, thanks for this patch!  My same thoughts about
> > > CONFIG_CC_IS_CLANG vs cc-option from
> > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-November/180939.html
> > > apply here as well.  I don't feel strongly about either though.  What
> > > are your thoughts?
> >
> > I'm not sure that we can test for this one with cc-option. The result
> > of having -maltivec with -msoft-float is a error about the internals
> > of clang, which isn't something that kbuild is set up to test for.
>
> As in clang itself crashes, and cc-option/kbuild can't handle that gracefully?

The developer gets something like this:

SplitVectorResult #0: t196: v16i8 = llvm.ppc.altivec.vcmpgtsb
TargetConstant:i64<4823>, t146, t195
fatal error: error in backend: Do not know how to split the result of
this operator!
clang-8: error: clang frontend command failed with exit code 70 (use
-v to see invocation)

>
> >
> > When clang is fixed to allow this combination we will still build this
> > code in the same way, so in that sense it fails "open".
> >

Re: [PATCH v2] raid6/ppc: Fix build for clang

[Nick Desaulniers]

On Mon, Dec 3, 2018 at 2:14 PM Joel Stanley <joel@jms.id.au> wrote:
>
> On Tue, 4 Dec 2018 at 05:15, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > > > +ifdef CONFIG_CC_IS_CLANG
> > > > > +# clang ppc port does not yet support -maltivec when -msoft-float is
> > > > > +# enabled. A future release of clang will resolve this
> > > > > +# https://bugs.llvm.org/show_bug.cgi?id=31177
> > > > > +CFLAGS_REMOVE_altivec1.o  += -msoft-float
> > > > > +CFLAGS_REMOVE_altivec2.o  += -msoft-float
> > > > > +CFLAGS_REMOVE_altivec4.o  += -msoft-float
> > > > > +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> > > > > +CFLAGS_REMOVE_altivec8.o  += -msoft-float
> > > > > +CFLAGS_REMOVE_vpermxor1.o += -msoft-float
> > > > > +CFLAGS_REMOVE_vpermxor2.o += -msoft-float
> > > > > +CFLAGS_REMOVE_vpermxor4.o += -msoft-float
> > > > > +CFLAGS_REMOVE_vpermxor8.o += -msoft-float
> > > > > +endif
> > > >
> > > > Hi Joel, thanks for this patch!  My same thoughts about
> > > > CONFIG_CC_IS_CLANG vs cc-option from
> > > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-November/180939.html
> > > > apply here as well.  I don't feel strongly about either though.  What
> > > > are your thoughts?
> > >
> > > I'm not sure that we can test for this one with cc-option. The result
> > > of having -maltivec with -msoft-float is a error about the internals
> > > of clang, which isn't something that kbuild is set up to test for.
> >
> > As in clang itself crashes, and cc-option/kbuild can't handle that gracefully?
>
> The developer gets something like this:
>
> SplitVectorResult #0: t196: v16i8 = llvm.ppc.altivec.vcmpgtsb
> TargetConstant:i64<4823>, t146, t195
> fatal error: error in backend: Do not know how to split the result of
> this operator!
> clang-8: error: clang frontend command failed with exit code 70 (use
> -v to see invocation)
>
> >
> > >
> > > When clang is fixed to allow this combination we will still build this
> > > code in the same way, so in that sense it fails "open".
> > >

Eek, that means cc-option is not hardened against flags that crash the
compiler (misbehaving compiler).  We'll have to get some version check
in place should we ever want to use those flags for these object
files, but for now:

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2] raid6/ppc: Fix build for clang

[Segher Boessenkool]

On Tue, Dec 04, 2018 at 08:43:47AM +1030, Joel Stanley wrote:
> On Tue, 4 Dec 2018 at 05:15, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > > > +ifdef CONFIG_CC_IS_CLANG
> > > > > +# clang ppc port does not yet support -maltivec when -msoft-float is
> > > > > +# enabled. A future release of clang will resolve this
> > > > > +# https://bugs.llvm.org/show_bug.cgi?id=31177

> > As in clang itself crashes, and cc-option/kbuild can't handle that gracefully?
> 
> The developer gets something like this:
> 
> SplitVectorResult #0: t196: v16i8 = llvm.ppc.altivec.vcmpgtsb
> TargetConstant:i64<4823>, t146, t195
> fatal error: error in backend: Do not know how to split the result of
> this operator!
> clang-8: error: clang frontend command failed with exit code 70 (use
> -v to see invocation)

-msoft-float simply means not to use FPRs, and nothing more or less, so
that is a pretty interesting failure (will probably turn out to be a typo
or something else boring, but it is fun while it lasts).


Segher

Re: [v2] raid6/ppc: Fix build for clang

[Michael Ellerman]

On Fri, 2018-11-02 at 00:44:55 UTC, Joel Stanley wrote:
> We cannot build these files with clang as it does not allow altivec
> instructions in assembly when -msoft-float is passed.
> 
> Jinsong Ji <jji@us.ibm.com> wrote:
> > We currently disable Altivec/VSX support when enabling soft-float.  So
> > any usage of vector builtins will break.
> >
> > Enable Altivec/VSX with soft-float may need quite some clean up work, so
> > I guess this is currently a limitation.
> >
> > Removing -msoft-float will make it work (and we are lucky that no
> > floating point instructions will be generated as well).
> 
> This is a workaround until the issue is resolved in clang.
> 
> Link: https://bugs.llvm.org/show_bug.cgi?id=31177
> Link: https://github.com/ClangBuiltLinux/linux/issues/239
> 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/e213574a449f7a57d4202c1869bbc7

cheers