| Message ID | 20181102004455.10157-1-joel@jms.id.au |
|---|---|
| State | Accepted |
| Commit | e213574a449f7a57d4202c1869bbc7680b6b5521 |
| Headers | show |
| Series | [v2] raid6/ppc: Fix build for clang | expand |
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
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
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
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". > >
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
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
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