2018-11-02 00:47:02

by Joel Stanley

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

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

Jinsong Ji <[email protected]> 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 <[email protected]>
---
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



2018-11-02 17:35:56

by Nick Desaulniers

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

On Thu, Nov 1, 2018 at 5:45 PM Joel Stanley <[email protected]> wrote:
>
> We cannot build these files with clang as it does not allow altivec
> instructions in assembly when -msoft-float is passed.
>
> Jinsong Ji <[email protected]> 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 <[email protected]>
> ---
> 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

2018-12-03 10:25:46

by Joel Stanley

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

On Sat, 3 Nov 2018 at 04:04, Nick Desaulniers <[email protected]> wrote:
>
> On Thu, Nov 1, 2018 at 5:45 PM Joel Stanley <[email protected]> wrote:
> >
> > We cannot build these files with clang as it does not allow altivec
> > instructions in assembly when -msoft-float is passed.
> >
> > Jinsong Ji <[email protected]> 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 <[email protected]>
> > ---
> > 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

2018-12-03 18:46:05

by Nick Desaulniers

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

On Mon, Dec 3, 2018 at 2:24 AM Joel Stanley <[email protected]> wrote:
>
> On Sat, 3 Nov 2018 at 04:04, Nick Desaulniers <[email protected]> wrote:
> >
> > On Thu, Nov 1, 2018 at 5:45 PM Joel Stanley <[email protected]> wrote:
> > >
> > > We cannot build these files with clang as it does not allow altivec
> > > instructions in assembly when -msoft-float is passed.
> > >
> > > Jinsong Ji <[email protected]> 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 <[email protected]>
> > > ---
> > > 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

2018-12-03 22:15:27

by Joel Stanley

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

On Tue, 4 Dec 2018 at 05:15, Nick Desaulniers <[email protected]> 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".
> >

2018-12-03 22:57:15

by Nick Desaulniers

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

On Mon, Dec 3, 2018 at 2:14 PM Joel Stanley <[email protected]> wrote:
>
> On Tue, 4 Dec 2018 at 05:15, Nick Desaulniers <[email protected]> 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 <[email protected]>
--
Thanks,
~Nick Desaulniers

2018-12-04 01:04:36

by Segher Boessenkool

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

On Tue, Dec 04, 2018 at 08:43:47AM +1030, Joel Stanley wrote:
> On Tue, 4 Dec 2018 at 05:15, Nick Desaulniers <[email protected]> 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

2018-12-23 11:03:08

by Michael Ellerman

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

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 <[email protected]> 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 <[email protected]>
> Reviewed-by: Nick Desaulniers <[email protected]>

Applied to powerpc next, thanks.

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

cheers