Re: [v5 1/2] lib/raid6: Build proper files on corresponding arch

[Matt Brown]

On Wed, Aug 2, 2017 at 12:00 PM, Michael Ellerman  wrote:
> Daniel Axtens  writes:
>
>> Hi Matt,
>>
>>> --- a/lib/raid6/test/Makefile
>>> +++ b/lib/raid6/test/Makefile
>>> @@ -44,10 +44,12 @@ else ifeq ($(HAS_NEON),yes)
>>>  CFLAGS += -DCONFIG_KERNEL_MODE_NEON=1
>>>  else
>>>  HAS_ALTIVEC := $(shell printf '\#include \nvector int 
>>> a;\n' |\
>>> - gcc -c -x c - >&/dev/null && \
>>> - rm ./-.o && echo yes)
>>> + gcc -c -x c - >/dev/null && rm ./-.o && echo yes)
>>
>> From memory the change here (s/>&/>/) was necessary to get the build to
>> succeed - did we ever figure out why that was? I'm not enough of a shell
>> guru to grok the difference.
>
> Using >& redirects stdout and stderr, whereas > only redirects stdout.
>
> So possibly it doesn't fix anything, but rather lets you see any error
> emitted by the compiler rather than swallowing it?
>

Just had to double-check what the problem was.
The bug was that none of the ppc specific files were being built.
I'm not entirely sure how, but this fixes it so the altivec and
vpermxor files are built.

I'll fix up the commit message and move the vpermxor make defs into
the other patch.

Thanks,
Matt

> cheers

Re: [v5 1/2] lib/raid6: Build proper files on corresponding arch

[Michael Ellerman]

Daniel Axtens  writes:

> Hi Matt,
>
>> --- a/lib/raid6/test/Makefile
>> +++ b/lib/raid6/test/Makefile
>> @@ -44,10 +44,12 @@ else ifeq ($(HAS_NEON),yes)
>>  CFLAGS += -DCONFIG_KERNEL_MODE_NEON=1
>>  else
>>  HAS_ALTIVEC := $(shell printf '\#include \nvector int 
>> a;\n' |\
>> - gcc -c -x c - >&/dev/null && \
>> - rm ./-.o && echo yes)
>> + gcc -c -x c - >/dev/null && rm ./-.o && echo yes)
>
> From memory the change here (s/>&/>/) was necessary to get the build to
> succeed - did we ever figure out why that was? I'm not enough of a shell
> guru to grok the difference.

Using >& redirects stdout and stderr, whereas > only redirects stdout.

So possibly it doesn't fix anything, but rather lets you see any error
emitted by the compiler rather than swallowing it?

cheers

Re: [v5 1/2] lib/raid6: Build proper files on corresponding arch

[Daniel Axtens]

Hi Matt,

> --- a/lib/raid6/test/Makefile
> +++ b/lib/raid6/test/Makefile
> @@ -44,10 +44,12 @@ else ifeq ($(HAS_NEON),yes)
>  CFLAGS += -DCONFIG_KERNEL_MODE_NEON=1
>  else
>  HAS_ALTIVEC := $(shell printf '\#include \nvector int 
> a;\n' |\
> - gcc -c -x c - >&/dev/null && \
> - rm ./-.o && echo yes)
> + gcc -c -x c - >/dev/null && rm ./-.o && echo yes)

>From memory the change here (s/>&/>/) was necessary to get the build to
succeed - did we ever figure out why that was? I'm not enough of a shell
guru to grok the difference. If it's easy to explain it would be good to
put it in the commit message, rather than just saying you fixed an
unspecified bug.

>  ifeq ($(HAS_ALTIVEC),yes)
> -OBJS += altivec1.o altivec2.o altivec4.o altivec8.o
> +CFLAGS += -I../../../arch/powerpc/include
> +CFLAGS += -DCONFIG_ALTIVEC
> +OBJS += altivec1.o altivec2.o altivec4.o altivec8.o \
> +vpermxor1.o vpermxor2.o vpermxor4.o vpermxor8.o

You've added vpermxor here, but you don't define them until the next
patch, so the tests will fail. Please move the change to OBJS to the
next patch.

With that change, I'd be happy to formally Review this patch.

Regards,
Daniel

>  endif
>  endif
>  ifeq ($(ARCH),tilegx)
> -- 
> 2.9.3

[v5 1/2] lib/raid6: Build proper files on corresponding arch

[Matt Brown]

Previously the raid6 test Makefile did not correctly build the files for
testing on PowerPC. This patch fixes the bug, so that all appropriate files
for PowerPC are built.
This patch also fixes the missing and mismatched ifdef statements to allow the
altivec.uc file to be built correctly.

Signed-off-by: Matt Brown 
---
Changelog
v5
- moved altivec.uc fix into this patch
- updates commit message
---
 lib/raid6/altivec.uc| 3 +++
 lib/raid6/test/Makefile | 8 +---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/raid6/altivec.uc b/lib/raid6/altivec.uc
index 682aae8..d20ed0d 100644
--- a/lib/raid6/altivec.uc
+++ b/lib/raid6/altivec.uc
@@ -24,10 +24,13 @@
 
 #include 
 
+#ifdef CONFIG_ALTIVEC
+
 #include 
 #ifdef __KERNEL__
 # include 
 # include 
+#endif /* __KERNEL__ */
 
 /*
  * This is the C data type to use.  We use a vector of
diff --git a/lib/raid6/test/Makefile b/lib/raid6/test/Makefile
index 9c333e9..b64a267 100644
--- a/lib/raid6/test/Makefile
+++ b/lib/raid6/test/Makefile
@@ -44,10 +44,12 @@ else ifeq ($(HAS_NEON),yes)
 CFLAGS += -DCONFIG_KERNEL_MODE_NEON=1
 else
 HAS_ALTIVEC := $(shell printf '\#include \nvector int a;\n' 
|\
- gcc -c -x c - >&/dev/null && \
- rm ./-.o && echo yes)
+ gcc -c -x c - >/dev/null && rm ./-.o && echo yes)
 ifeq ($(HAS_ALTIVEC),yes)
-OBJS += altivec1.o altivec2.o altivec4.o altivec8.o
+CFLAGS += -I../../../arch/powerpc/include
+CFLAGS += -DCONFIG_ALTIVEC
+OBJS += altivec1.o altivec2.o altivec4.o altivec8.o \
+vpermxor1.o vpermxor2.o vpermxor4.o vpermxor8.o
 endif
 endif
 ifeq ($(ARCH),tilegx)
-- 
2.9.3