opus - Feb 2015 - [RFC PATCH v2] Encode optimize using libNe10

Viswanath Puttagunta wrote:
> Can we please have review on RFCv2? We have quite a few optimizations
> (Eg: ifft/mdct_backwards, fixed point fft/ifft mdct_forward/backward
> etc) that are in my pipeline that depend on this patch series being
> accepted.
So, trying to make progress on this...

On an armv7l board running Ubuntu, you've broken the build with just 
--enable-intrinsics (without the NE10 options):

   CC       celt/tests/test_unit_dft.o
In file included from ../celt/tests/test_unit_dft.c:47:0:
../celt/kiss_fft.c: In function 'opus_fft_free':
../celt/kiss_fft.c:507:51: warning: unused parameter 'arch' 
[-Wunused-parameter]
  void opus_fft_free(const kiss_fft_state *cfg, int arch)
                                                    ^
   CCLD     celt/tests/test_unit_dft
celt/tests/test_unit_dft.o: In function `_celt_autocorr':
/home/tterribe/src/xiph/git/xiph/opus/build/../celt/celt_lpc.c:285: 
undefined reference to `CELT_PITCH_XCORR_IMPL'
/home/tterribe/src/xiph/git/xiph/opus/build/../celt/celt_lpc.c:285: 
undefined reference to `CELT_PITCH_XCORR_IMPL'
celt/tests/test_unit_dft.o: In function `pitch_search':
/home/tterribe/src/xiph/git/xiph/opus/build/../celt/pitch.c:358: 
undefined reference to `CELT_PITCH_XCORR_IMPL'
/home/tterribe/src/xiph/git/xiph/opus/build/../celt/pitch.c:358: 
undefined reference to `CELT_PITCH_XCORR_IMPL'
collect2: error: ld returned 1 exit status


The configure test fails with the NE10 options, because the test program 
is not being linked against $LIBM:

configure:13471: gcc -std=gnu99 -o conftest 
-I/home/tterribe/src/xiph/git/arm/Ne10/inc   conftest.c 
-L/home/tterribe/src/xiph/git/arm/Ne10/build/modules -lNE10 >&5
/home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a(NE10_fft.c.o): 
In function `ne10_fft_generate_twiddles_line_int32':
NE10_fft.c:(.text+0x6e): undefined reference to `sincos'
NE10_fft.c:(.text+0x82): undefined reference to `floor'
NE10_fft.c:(.text+0x96): undefined reference to `floor'
/home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a(NE10_fft.c.o): 
In function `ne10_fft_generate_twiddles_line_float32':
NE10_fft.c:(.text+0x138): undefined reference to `sincos'
collect2: error: ld returned 1 exit status


Forcing it to be enabled, linking against libNE10 fails:

   CCLD     libopus.la
/usr/bin/ld: 
/home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a(NE10_fft_generic_float32.neonintrinsic.cpp.o):
relocation R_ARM_THM_MOVW_ABS_NC against `a local symbol' can not be 
used when making a shared object; recompile with -fPIC
/home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a: error 
adding symbols: Bad value
collect2: error: ld returned 1 exit status


Then attempting to build libNE10 with -DNE10_BUILD_SHARED fails:

/usr/bin/ld: CMakeFiles/NE10_shared.dir/imgproc/NE10_boxfilter.c.o: 
relocation R_ARM_THM_MOVW_ABS_NC against `stderr' can not be used when 
making a shared object; recompile with -fPIC
CMakeFiles/NE10_shared.dir/imgproc/NE10_boxfilter.c.o: error adding 
symbols: Bad value
collect2: error: ld returned 1 exit status


The instructions for building libNE10 in doc/BuildingNe10.txt are also 
incorrect. I don't know if -DGNULINUX_PLATFORM=ON does anything, but I 
needed to specify -DNE10_LINUX_TARGET_ARCH=armv7 to avoid cmake failing 
with a 'syntax error' at line 83.


In short, this needs some work.

On 25 February 2015 at 19:54, Timothy B. Terriberry <tterribe at xiph.org>
wrote:
> Viswanath Puttagunta wrote:
>>
>> Can we please have review on RFCv2? We have quite a few optimizations
>> (Eg: ifft/mdct_backwards, fixed point fft/ifft mdct_forward/backward
>> etc) that are in my pipeline that depend on this patch series being
>> accepted.
>
>
> So, trying to make progress on this...
>
> On an armv7l board running Ubuntu, you've broken the build with just
> --enable-intrinsics (without the NE10 options):
Yep, I should have checked with just this option to make sure I'm not
causing regression. I will add to my test plan going forward.
>
>   CC       celt/tests/test_unit_dft.o
> In file included from ../celt/tests/test_unit_dft.c:47:0:
> ../celt/kiss_fft.c: In function 'opus_fft_free':
> ../celt/kiss_fft.c:507:51: warning: unused parameter 'arch'
> [-Wunused-parameter]
>  void opus_fft_free(const kiss_fft_state *cfg, int arch)
>                                                    ^
>   CCLD     celt/tests/test_unit_dft
> celt/tests/test_unit_dft.o: In function `_celt_autocorr':
> /home/tterribe/src/xiph/git/xiph/opus/build/../celt/celt_lpc.c:285:
> undefined reference to `CELT_PITCH_XCORR_IMPL'
> /home/tterribe/src/xiph/git/xiph/opus/build/../celt/celt_lpc.c:285:
> undefined reference to `CELT_PITCH_XCORR_IMPL'
> celt/tests/test_unit_dft.o: In function `pitch_search':
> /home/tterribe/src/xiph/git/xiph/opus/build/../celt/pitch.c:358: undefined
> reference to `CELT_PITCH_XCORR_IMPL'
> /home/tterribe/src/xiph/git/xiph/opus/build/../celt/pitch.c:358: undefined
> reference to `CELT_PITCH_XCORR_IMPL'
> collect2: error: ld returned 1 exit status
>
>
> The configure test fails with the NE10 options, because the test program is
> not being linked against $LIBM:
Can you please provide the exact configure command you are using? I
did not see this artifact. (However, I will check this again tomorrow)

Below is the configure command I use
../opus/configure --host=arm-linux-gnueabihf --enable-intrinsics
--with-NE10-libraries=<path>/armv7_root/binary/usr/local/lib
--with-NE10-includes=<path>//armv7_root/binary/usr/local/include/Ne10

I built libNe10 to be dynamic library and place libNe10.so* in
<path>/usr/local/lib/

and include files (copied from NE10 source "inc/" directory into
<path>usr/local/include/Ne10/
>
> configure:13471: gcc -std=gnu99 -o conftest
> -I/home/tterribe/src/xiph/git/arm/Ne10/inc   conftest.c
> -L/home/tterribe/src/xiph/git/arm/Ne10/build/modules -lNE10 >&5
> /home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a(NE10_fft.c.o):
> In function `ne10_fft_generate_twiddles_line_int32':
> NE10_fft.c:(.text+0x6e): undefined reference to `sincos'
> NE10_fft.c:(.text+0x82): undefined reference to `floor'
> NE10_fft.c:(.text+0x96): undefined reference to `floor'
> /home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a(NE10_fft.c.o):
> In function `ne10_fft_generate_twiddles_line_float32':
> NE10_fft.c:(.text+0x138): undefined reference to `sincos'
> collect2: error: ld returned 1 exit status
>
>
> Forcing it to be enabled, linking against libNE10 fails:
>
>   CCLD     libopus.la
> /usr/bin/ld:
>
/home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a(NE10_fft_generic_float32.neonintrinsic.cpp.o):
> relocation R_ARM_THM_MOVW_ABS_NC against `a local symbol' can not be
used
> when making a shared object; recompile with -fPIC
> /home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a: error adding
> symbols: Bad value
> collect2: error: ld returned 1 exit status
>
>
> Then attempting to build libNE10 with -DNE10_BUILD_SHARED fails:
>
> /usr/bin/ld: CMakeFiles/NE10_shared.dir/imgproc/NE10_boxfilter.c.o:
> relocation R_ARM_THM_MOVW_ABS_NC against `stderr' can not be used when
> making a shared object; recompile with -fPIC
> CMakeFiles/NE10_shared.dir/imgproc/NE10_boxfilter.c.o: error adding
symbols:
> Bad value
> collect2: error: ld returned 1 exit status
>
>
> The instructions for building libNE10 in doc/BuildingNe10.txt are also
> incorrect. I don't know if -DGNULINUX_PLATFORM=ON does anything, but I
> needed to specify -DNE10_LINUX_TARGET_ARCH=armv7 to avoid cmake failing
with
> a 'syntax error' at line 83.
I use the following commands
cmake -DNE10_BUILD_SHARED=ON -DCMAKE_TOOLCHAIN_FILE=../
GNUlinux_config.cmake ..

The GNUlinux_config.cmake sets up any other variables. I will update
this in libNE10.
>
>
> In short, this needs some work.
Yep, Will get back to you by end of this week at the latest.

Viswanath Puttagunta wrote:
> Can you please provide the exact configure command you are using? I
> did not see this artifact. (However, I will check this again tomorrow)
>
> Below is the configure command I use
> ../opus/configure --host=arm-linux-gnueabihf --enable-intrinsics
> --with-NE10-libraries=<path>/armv7_root/binary/usr/local/lib
> --with-NE10-includes=<path>//armv7_root/binary/usr/local/include/Ne10
configure --enable-intrinsics 
--with-NE10-libraries=$HOME/src/xiph/git/arm/Ne10/build/modules 
--with-NE10-includes=$HOME/src/xiph/git/arm/Ne10/inc

(building directly on the device, not cross-compiling)

Hello Timothy,

I was able to fix the compile issue without NE10 (just with
--enable-intrinsics).

However, I am unable to reproduce your failures when you enable NE10.
For me, it builds without any compile or linker errors. (Your
configure command looks fine to me)

I tried both cross compile (4.9 gcc linaro binaries)
   build log: https://pastebin.linaro.org/view/92f4d1f7
and
native compile (4.8 gcc based- installed with apt-get install build-essentials)
    build log: https://pastebin.linaro.org/view/115a80a9

I'm wondering if issue you faced when building with NE10 are an
artifact of improperly built libNE10.so.

You can see my working opus repo at
https://git.linaro.org/people/viswanath.puttagunta/opus.git
Branch: rfcv3_rc1_opus_fft_ne10
(This has the compile issue you pointed to fixed)

For your convinience, I have libNE10.so* and include files here
http://people.linaro.org/~viswanath.puttagunta/opus/NE10_root/NE10_root.tar.gz

Can you please try to compile with above library and includes and see
if you see same issue? I would like to narrow down where the issue is.

Regards,
Vish



On 25 February 2015 at 21:22, Viswanath Puttagunta
<viswanath.puttagunta at linaro.org> wrote:
>
> On 25 February 2015 at 19:54, Timothy B. Terriberry <tterribe at
xiph.org> wrote:
> > Viswanath Puttagunta wrote:
> >>
> >> Can we please have review on RFCv2? We have quite a few
optimizations
> >> (Eg: ifft/mdct_backwards, fixed point fft/ifft
mdct_forward/backward
> >> etc) that are in my pipeline that depend on this patch series
being
> >> accepted.
> >
> >
> > So, trying to make progress on this...
> >
> > On an armv7l board running Ubuntu, you've broken the build with
just
> > --enable-intrinsics (without the NE10 options):
> Yep, I should have checked with just this option to make sure I'm not
> causing regression. I will add to my test plan going forward.
>
> >
> >   CC       celt/tests/test_unit_dft.o
> > In file included from ../celt/tests/test_unit_dft.c:47:0:
> > ../celt/kiss_fft.c: In function 'opus_fft_free':
> > ../celt/kiss_fft.c:507:51: warning: unused parameter 'arch'
> > [-Wunused-parameter]
> >  void opus_fft_free(const kiss_fft_state *cfg, int arch)
> >                                                    ^
> >   CCLD     celt/tests/test_unit_dft
> > celt/tests/test_unit_dft.o: In function `_celt_autocorr':
> > /home/tterribe/src/xiph/git/xiph/opus/build/../celt/celt_lpc.c:285:
> > undefined reference to `CELT_PITCH_XCORR_IMPL'
> > /home/tterribe/src/xiph/git/xiph/opus/build/../celt/celt_lpc.c:285:
> > undefined reference to `CELT_PITCH_XCORR_IMPL'
> > celt/tests/test_unit_dft.o: In function `pitch_search':
> > /home/tterribe/src/xiph/git/xiph/opus/build/../celt/pitch.c:358:
undefined
> > reference to `CELT_PITCH_XCORR_IMPL'
> > /home/tterribe/src/xiph/git/xiph/opus/build/../celt/pitch.c:358:
undefined
> > reference to `CELT_PITCH_XCORR_IMPL'
> > collect2: error: ld returned 1 exit status
> >
> >
> > The configure test fails with the NE10 options, because the test
program is
> > not being linked against $LIBM:
>
> Can you please provide the exact configure command you are using? I
> did not see this artifact. (However, I will check this again tomorrow)
>
> Below is the configure command I use
> ../opus/configure --host=arm-linux-gnueabihf --enable-intrinsics
> --with-NE10-libraries=<path>/armv7_root/binary/usr/local/lib
> --with-NE10-includes=<path>//armv7_root/binary/usr/local/include/Ne10
>
> I built libNe10 to be dynamic library and place libNe10.so* in
> <path>/usr/local/lib/
>
> and include files (copied from NE10 source "inc/" directory into
> <path>usr/local/include/Ne10/
>
> >
> > configure:13471: gcc -std=gnu99 -o conftest
> > -I/home/tterribe/src/xiph/git/arm/Ne10/inc   conftest.c
> > -L/home/tterribe/src/xiph/git/arm/Ne10/build/modules -lNE10 >&5
> >
/home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a(NE10_fft.c.o):
> > In function `ne10_fft_generate_twiddles_line_int32':
> > NE10_fft.c:(.text+0x6e): undefined reference to `sincos'
> > NE10_fft.c:(.text+0x82): undefined reference to `floor'
> > NE10_fft.c:(.text+0x96): undefined reference to `floor'
> >
/home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a(NE10_fft.c.o):
> > In function `ne10_fft_generate_twiddles_line_float32':
> > NE10_fft.c:(.text+0x138): undefined reference to `sincos'
> > collect2: error: ld returned 1 exit status
> >
> >
> > Forcing it to be enabled, linking against libNE10 fails:
> >
> >   CCLD     libopus.la
> > /usr/bin/ld:
> >
/home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a(NE10_fft_generic_float32.neonintrinsic.cpp.o):
> > relocation R_ARM_THM_MOVW_ABS_NC against `a local symbol' can not
be used
> > when making a shared object; recompile with -fPIC
> > /home/tterribe/src/xiph/git/arm/Ne10/build/modules/libNE10.a: error
adding
> > symbols: Bad value
> > collect2: error: ld returned 1 exit status
> >
> >
> > Then attempting to build libNE10 with -DNE10_BUILD_SHARED fails:
> >
> > /usr/bin/ld: CMakeFiles/NE10_shared.dir/imgproc/NE10_boxfilter.c.o:
> > relocation R_ARM_THM_MOVW_ABS_NC against `stderr' can not be used
when
> > making a shared object; recompile with -fPIC
> > CMakeFiles/NE10_shared.dir/imgproc/NE10_boxfilter.c.o: error adding
symbols:
> > Bad value
> > collect2: error: ld returned 1 exit status
> >
> >
> > The instructions for building libNE10 in doc/BuildingNe10.txt are also
> > incorrect. I don't know if -DGNULINUX_PLATFORM=ON does anything,
but I
> > needed to specify -DNE10_LINUX_TARGET_ARCH=armv7 to avoid cmake
failing with
> > a 'syntax error' at line 83.
>
> I use the following commands
> cmake -DNE10_BUILD_SHARED=ON -DCMAKE_TOOLCHAIN_FILE=../
> GNUlinux_config.cmake ..
>
> The GNUlinux_config.cmake sets up any other variables. I will update
> this in libNE10.
>
> >
> >
> > In short, this needs some work.
> Yep, Will get back to you by end of this week at the latest.