Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken Altivec support #84

Closed
dhowland opened this issue Mar 29, 2017 · 12 comments
Closed

Broken Altivec support #84

dhowland opened this issue Mar 29, 2017 · 12 comments

Comments

@dhowland
Copy link

Trying to compile Liquid for modern PPC targets with the NXP SDK:

user@host:~/dev/liquid-dsp$ make
powerpc-fsl-linux-gcc  -m32 -mhard-float -mcpu=e6500 --sysroot=/opt/fsl-qoriq/2.0/sysroots/ppce6500-fsl-linux  -O2 -pipe -g -feliminate-unused-debug-types -I . -I include -Wall -fPIC -O2 -pipe -g -feliminate-unused-debug-types  -fno-common -faltivec   -c -o src/libliquid.o src/libliquid.c
powerpc-fsl-linux-gcc: error: unrecognized command line option '-faltivec'
<builtin>: recipe for target 'src/libliquid.o' failed
make: *** [src/libliquid.o] Error 1

Perhaps -faltivec is a Mac thing. It appears that GCC uses -maltivec -mabi=altivec. I know autoconfigure has the ability to discover the correct flags, because FFTW does so. I changed the makefile and was able to continue with the compilation. However, it failed at the dotprod module thus:

powerpc-fsl-linux-gcc  -m32 -mhard-float -mcpu=e6500 --sysroot=/opt/fsl-qoriq/2.0/sysroots/ppce6500-fsl-linux  -O2 -pipe -g -feliminate-unused-debug-types -I . -I include -Wall -fPIC -O2 -pipe -g -feliminate-unused-debug-types  -fno-common -maltivec -mabi=altivec   -c -o src/dotprod/src/dotprod_rrrf.av.o src/dotprod/src/dotprod_rrrf.av.c
src/dotprod/src/dotprod_rrrf.av.c: In function 'dotprod_rrrf_execute':
src/dotprod/src/dotprod_rrrf.av.c:176:5: error: can't convert between vector values of different size
     s0 = s1 = s2 = s3 = (vector float)(0);
     ^
src/dotprod/src/dotprod_rrrf.av.c:178:9: warning: implicit declaration of function 'vec_madd' [-Wimplicit-function-declaration]
         s0 = vec_madd(ar[nblocks-1],d[nblocks-1],s0);
         ^
src/dotprod/src/dotprod_rrrf.av.c:178:9: error: AltiVec argument passed to unprototyped function
src/dotprod/src/dotprod_rrrf.av.c:179:9: error: AltiVec argument passed to unprototyped function
         s1 = vec_madd(ar[nblocks-2],d[nblocks-2],s1);
         ^
src/dotprod/src/dotprod_rrrf.av.c:180:9: error: AltiVec argument passed to unprototyped function
         s2 = vec_madd(ar[nblocks-3],d[nblocks-3],s2);
         ^
src/dotprod/src/dotprod_rrrf.av.c:181:9: error: AltiVec argument passed to unprototyped function
         s3 = vec_madd(ar[nblocks-4],d[nblocks-4],s3);
         ^
src/dotprod/src/dotprod_rrrf.av.c:186:5: warning: implicit declaration of function 'vec_add' [-Wimplicit-function-declaration]
     s0 = vec_add(s0,s1);    // s0 = s0+s1
     ^
src/dotprod/src/dotprod_rrrf.av.c:186:5: error: AltiVec argument passed to unprototyped function
src/dotprod/src/dotprod_rrrf.av.c:187:5: error: AltiVec argument passed to unprototyped function
     s2 = vec_add(s2,s3);    // s2 = s2+s3
     ^
src/dotprod/src/dotprod_rrrf.av.c:188:5: error: AltiVec argument passed to unprototyped function
     s0 = vec_add(s0,s2);    // s0 = s0+s2
     ^
src/dotprod/src/dotprod_rrrf.av.c:192:9: error: AltiVec argument passed to unprototyped function
         s0 = vec_madd(ar[nblocks],d[nblocks],s0);
         ^
src/dotprod/src/dotprod_rrrf.av.c:197:5: error: can't convert between vector values of different size
     s.v = vec_add(s0,(vector float)(0));
     ^
src/dotprod/src/dotprod_rrrf.av.c:197:5: error: AltiVec argument passed to unprototyped function
<builtin>: recipe for target 'src/dotprod/src/dotprod_rrrf.av.o' failed
make: *** [src/dotprod/src/dotprod_rrrf.av.o] Error 1

This code doesn't even have #include "altivec.h". Not sure how that was expected to work...
Anyway I added that but a few of the errors remain.

I am currently working on fixing the code, I will offer a pull request if I get it working.

One more question:

Why is LIQUID_IIRFILT_USE_DOTPROD defined as 0 in iirfilt? If I can use SIMD acceleration for my filters, I'd like to do that.

thanks.

@dhowland
Copy link
Author

Okay I got it compiling by changing the flags, including altivec.h, and following the documentation for gc, here: https://gcc.gnu.org/onlinedocs/gcc-4.3.2/gcc/PowerPC-AltiVec-Built_002din-Functions.html

Basically, you can't just do (vector float)(0) in the gnu tools. You have to use ((vector float){0, 0, 0, 0})

Still wondering about the LIQUID_IIRFILT_USE_DOTPROD, though

@jgaeddert
Copy link
Owner

I used to have a PowerBook G4 way back in the day and that's where the AltiVec code was tested on originally. I don't have a PPC machine any more so I can't easily test AV extensions now, but I believe you when you say your fix works.

As for not using the vectorized dot product with the IIR filter, I can't remember why I disabled it. I can dig into that to see if there were any issues with it. Try enabling them and then running "make check" to see if it passes the auto tests.

@dhowland
Copy link
Author

That will be tough because I'm cross compiling and my target is an embedded system, but I'll check it out.

@dhowland
Copy link
Author

Okay, well at least on x86 there was a problem:

47: iirfilt_xxxf
makefile:1274: recipe for target 'check' failed
make: *** [check] Segmentation fault (core dumped)

I'll get out the debugger

@dhowland
Copy link
Author

dhowland commented Mar 30, 2017

Oh, I figured out why LIQUID_IIRFILT_USE_DOTPROD is disabled. It's because only the normal filter functions support it. The SOS-related functions don't. The create functions were different but the destroy function was common. It led to mismatched malloc/free calls. I added some code and it passed the check.

@jgaeddert
Copy link
Owner

I can reproduce the double-free bug on my side. I committed a fix. I also defaulted to using vectorized dot product with non-SOS methods. I'm seeing a speed improvement of about 2x. Looking at how easy it would be to convert SOS to using vectorized dot product now.

@jgaeddert
Copy link
Owner

I added a preprocessor macro (LIQUID_IIRFILTSOS_USE_DOTPROD) for using vectorized dot product methods for second-order sections with the iirfilt family. In general, though, it's slower because of overhead. I defaulted to not enable this, but I did make sure it is functionally correct. It might be useful to investigate speed improvements for the iirfilt SOS methods, but for now I think it's generally best to use the SOS with non-vectorized methods.

@dhowland
Copy link
Author

dhowland commented Apr 1, 2017

Oh wow, thanks. Interesting stuff. Your code definitely goes further than mine did.

I will put a branch with changes to support Altivec on GCC somewhere so you can get to it if you need it.

@mdomsch
Copy link

mdomsch commented Aug 15, 2018

current HEAD fails to build on Fedora "rawhide" ppc64le as -faltivec isn't supported in gcc there.
gcc 8.2.1-2.fc29.
Any advice welcome.

@sharkcz
Copy link

sharkcz commented Aug 15, 2018

AFAIK the gcc option is -maltivec, -std=gnu++11 might be needed as well to solve some bool/vector type conflicts. I'll give it a try on my system.

@sharkcz
Copy link

sharkcz commented Aug 15, 2018

After adding #include <altivec.h> in dotprod_rrrf.av.c I get

[dan@talos liquid-dsp]$ gcc -g -O2  -fno-common -maltivec -Wall -fPIC -I. -Iinclude  -c -o src/dotprod/src/dotprod_rrrf.av.o src/dotprod/src/dotprod_rrrf.av.c                          
src/dotprod/src/dotprod_rrrf.av.c: In function ‘dotprod_rrrf_execute’:
src/dotprod/src/dotprod_rrrf.av.c:167:27: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     ar = (vector float*)( (int)_x & ~15);
                           ^
src/dotprod/src/dotprod_rrrf.av.c:167:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     ar = (vector float*)( (int)_x & ~15);
          ^
src/dotprod/src/dotprod_rrrf.av.c:168:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     al = ((int)_x & 15)/sizeof(float);
           ^
src/dotprod/src/dotprod_rrrf.av.c:177:5: error: can’t convert a value of type ‘int’ to vector type ‘__vector(4) float’ which has different size
     s0 = s1 = s2 = s3 = (vector float)(0);
     ^~
src/dotprod/src/dotprod_rrrf.av.c:198:5: error: can’t convert a value of type ‘int’ to vector type ‘__vector(4) float’ which has different size
     s.v = vec_add(s0,(vector float)(0));
     ^

which sounds like a problem with 32 bit (the code expectation) versus 64 bit (the system).

@sharkcz
Copy link

sharkcz commented Aug 16, 2018

fix proposed in #136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants