heavy on newest bela release

Llokki · May 1, 2019

hi there, is it worthwhile to update to the newest release, since that includes heavy on the bela image? and i see we don't have to use gcc anymore, is there a performance benefit in doing that? right now i use heavy on board but i have to compile with:

root@bela:~# make -C ~/Bela PROJECT=granular COMPILER=gcc run

giuliomoro · May 1, 2019

lokki hi there, is it worthwhile to update to the newest release, since that includes heavy on the bela image?

I wouldn't say so. Including it on the image is as simple as copying it over, as I think you did before.

Actually the fix that allows to use clang (this one) is not even on the most recent Bela image. clang is definitely faster at compiling than gcc, but I have not checked the performance of the generated code recently. In the past (gcc4.9 / clang3.8), the "Techno world" patch would have the following CPU usage:

libpd                62%
heavy/gcc       39% 
heavy/clang    29%

Llokki · May 1, 2019

so i simply issue a

make -C ~/Bela PROJECT=granular  run

and if this works i am good to go...

giuliomoro · May 1, 2019

Llokki · May 1, 2019

ok, i get some errors. i have now changed the HvLightPipe.c manually, i still had the unpatched version. on to recompile and see if it works.

Llokki · May 1, 2019

getting further HvLightPipe compiles, but at the end i get:

Building render.cpp...
 ...done
 
Linking...
 ...done
Running /root/Bela/projects/granular/granular 
__hv_noteout: 0xd1d4ac2
__hv_ctlout: 0xe5e2a040
__hv_pgmout: 0x8753e39e
__hv_touchout: 0x476d4387
__hv_polytouchout: 0xd5aca9d1
__hv_bendout: 0xe8458013
__hv_midiout: 0x6511de55
: 0
Bus error
Makefile:515: recipe for target 'runonly' failed
make: *** [runonly] Error 135
make: Leaving directory '/root/Bela'

giuliomoro · May 1, 2019

Better clean the project and rebuild only with clang

Llokki · May 1, 2019

did that already. still the same error. maybe it is time to checkout the hvcc from your repo.

giuliomoro · May 1, 2019

You could also try to clean and compile with CPPFLAGS="-O0 -g" then run the project inside gdb and see if that points out something useful about the error.
Also, I hope you are on a >= v0.3 Bela image, right?

Llokki · May 1, 2019

giuliomoro Also, I hope you are on a >= v0.3 Bela image, right?

yes i am on v 0.36

Llokki · May 1, 2019

the python part stays the same i guess:

python2.7 ~/hvcc/hvcc.py ~/Bela/projects/granular_patch/_main.pd -o /tmp/hvtmp -n bela -g c

giuliomoro · May 1, 2019

Yes. Can you try with a simpler patch?

Llokki · May 1, 2019

also i get a ton of these warnings (excerpt):

/root/Bela/projects/granular/Heavy_bela.hpp:5173:16: warning: private field 'cBinop_91Yd6GIf' is not used [-Wunused-private-field]
  ControlBinop cBinop_91Yd6GIf;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5174:16: warning: private field 'cBinop_xM9L6rtB' is not used [-Wunused-private-field]
  ControlBinop cBinop_xM9L6rtB;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5175:16: warning: private field 'cBinop_iXLNeOBL' is not used [-Wunused-private-field]
  ControlBinop cBinop_iXLNeOBL;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5176:16: warning: private field 'cBinop_MKGD40Kf' is not used [-Wunused-private-field]
  ControlBinop cBinop_MKGD40Kf;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5177:16: warning: private field 'cBinop_rIZAlgWU' is not used [-Wunused-private-field]
  ControlBinop cBinop_rIZAlgWU;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5184:16: warning: private field 'cBinop_Gvg4XdPM' is not used [-Wunused-private-field]
  ControlBinop cBinop_Gvg4XdPM;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5185:16: warning: private field 'cBinop_X2VsXBKa' is not used [-Wunused-private-field]
  ControlBinop cBinop_X2VsXBKa;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5186:16: warning: private field 'cBinop_YkTDTmsr' is not used [-Wunused-private-field]
  ControlBinop cBinop_YkTDTmsr;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5187:16: warning: private field 'cBinop_WNywptxP' is not used [-Wunused-private-field]
  ControlBinop cBinop_WNywptxP;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5197:16: warning: private field 'cBinop_iVY3cZQE' is not used [-Wunused-private-field]
  ControlBinop cBinop_iVY3cZQE;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5200:16: warning: private field 'cBinop_DG7GtOCA' is not used [-Wunused-private-field]
  ControlBinop cBinop_DG7GtOCA;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5204:16: warning: private field 'cBinop_W54Xo401' is not used [-Wunused-private-field]
  ControlBinop cBinop_W54Xo401;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5210:16: warning: private field 'cBinop_dXTJbKu0' is not used [-Wunused-private-field]
  ControlBinop cBinop_dXTJbKu0;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5211:16: warning: private field 'cBinop_WfQujrjE' is not used [-Wunused-private-field]
  ControlBinop cBinop_WfQujrjE;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5217:16: warning: private field 'cBinop_cFVLi9DT' is not used [-Wunused-private-field]
  ControlBinop cBinop_cFVLi9DT;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5233:16: warning: private field 'cBinop_6S60AzrN' is not used [-Wunused-private-field]
  ControlBinop cBinop_6S60AzrN;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5235:16: warning: private field 'cBinop_BOCunN0Z' is not used [-Wunused-private-field]
  ControlBinop cBinop_BOCunN0Z;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5237:16: warning: private field 'cBinop_qZxqM3zS' is not used [-Wunused-private-field]
  ControlBinop cBinop_qZxqM3zS;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5241:16: warning: private field 'cBinop_Xt9AFzM7' is not used [-Wunused-private-field]
  ControlBinop cBinop_Xt9AFzM7;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5242:16: warning: private field 'cBinop_cY9pW9P2' is not used [-Wunused-private-field]
  ControlBinop cBinop_cY9pW9P2;
               ^
/root/Bela/projects/granular/Heavy_bela.hpp:5254:16: warning: private field 'cBinop_wXhMB50h' is not used [-Wunused-private-field]
  ControlBinop cBinop_wXhMB50h;
               ^
428 warnings generated.
 ...done

Llokki · May 1, 2019

yes will try with something simpler. i am now compiling again with gcc and the warnings are very different, i get these:

Building Heavy_bela.cpp...
In file included from /root/Bela/projects/granular/Heavy_bela.hpp:43:0,
                 from /root/Bela/projects/granular/Heavy_bela.cpp:33:
/root/Bela/projects/granular/HvSignalTabread.h: In function 'void __hv_tabread_stoppable_f(SignalTabread*, float32x4_t*)':
/root/Bela/projects/granular/HvSignalTabread.h:134:15: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if (o->head == ~0x0) {
       ~~~~~~~~^~~~~~~
In file included from /root/Bela/projects/granular/Heavy_bela.hpp:44:0,
                 from /root/Bela/projects/granular/Heavy_bela.cpp:33:
/root/Bela/projects/granular/HvSignalTabwrite.h: In function 'void __hv_tabwrite_stoppable_f(SignalTabwrite*, float32x4_t)':
/root/Bela/projects/granular/HvSignalTabwrite.h:77:15: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if (o->head != HV_TABWRITE_STOPPED) {

which are not present in clang (but instead the ones above)

Llokki · May 1, 2019

ok. a simpler patch compiles and runs fine...

Llokki · May 1, 2019

ok i have a patch with just a reverb abstraction that fails... same Bus error. i use this reverb in the other patch as well.

so, to investigate further i compile with the flags you wrote and then how do i run inside gdb?

Llokki · May 1, 2019

ok i figured it out with gdb, i get this after i try to run the reverb patch inside gdb:

(gdb) run
Starting program: /root/Bela/projects/reverb/reverb 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
[New Thread 0xb693d450 (LWP 2621)]
__hv_noteout: 0xd1d4ac2
__hv_ctlout: 0xe5e2a040
__hv_pgmout: 0x8753e39e
__hv_touchout: 0x476d4387
__hv_polytouchout: 0xd5aca9d1
__hv_bendout: 0xe8458013
__hv_midiout: 0x6511de55
: 0

Thread 1 "reverb" received signal SIGBUS, Bus error.
0x00041e48 in sLine_init ()
(gdb)

sorry, i need a little assistance now :-)

giuliomoro · May 1, 2019

list

will tell you where that happened.

giuliomoro · May 1, 2019

You can also send me the patch and I will try to have a look. I am very afraid this could be something as horrible to track down as this one, and equally impossible to solve (as it was a bug in clang).

Llokki · May 1, 2019

giuliomoro

OK, will do when i get home. is it easier for you with a simpler patch or can i send the original patch?

giuliomoro · May 1, 2019

the smallest one that fail!

Llokki · May 2, 2019

if i try "list" in gdb i get:

1	../sysdeps/arm/crti.S: No such file or directory.

giuliomoro · May 2, 2019

backtrace , then frame x to move the frame x of the ones listed. I will have a look at what you sent me later today.

Llokki · May 2, 2019

i don't really have an idea what this tells me...but i get the feeling this is not to promising

root@bela:~/Bela/projects/reverb# gdb reverb
GNU gdb (Debian 7.12-6) 7.12.0.20161007-git
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "arm-linux-gnueabihf".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from reverb...done.
(gdb) backtrace
No stack.
(gdb) run
Starting program: /root/Bela/projects/reverb/reverb 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
[New Thread 0xb693d450 (LWP 822)]
__hv_noteout: 0xd1d4ac2
__hv_ctlout: 0xe5e2a040
__hv_pgmout: 0x8753e39e
__hv_touchout: 0x476d4387
__hv_polytouchout: 0xd5aca9d1
__hv_bendout: 0xe8458013
__hv_midiout: 0x6511de55
: 0

Thread 1 "reverb" received signal SIGBUS, Bus error.
0x00041e48 in sLine_init ()
(gdb) list
1	../sysdeps/arm/crti.S: No such file or directory.
(gdb) backtrace
#0  0x00041e48 in sLine_init ()
#1  0x0004474c in Heavy_bela::Heavy_bela (this=0xa2e58, sampleRate=44100, 
    poolKb=10, inQueueKb=2, outQueueKb=0)
    at /root/Bela/projects/reverb/Heavy_bela.cpp:66
#2  0x000446c8 in hv_bela_new_with_options (sampleRate=44100, poolKb=10, 
    inQueueKb=2, outQueueKb=0) at /root/Bela/projects/reverb/Heavy_bela.cpp:50
#3  0x00067c14 in setup (context=0x8a9f0 <gContext>, userData=0x0)
    at /root/Bela/projects/reverb/render.cpp:399
#4  0x00026b3c in Bela_initAudio ()
#5  0x00019312 in main ()
Warning: the current language does not match this frame.
(gdb) frame 0
#0  0x00041e48 in sLine_init ()
(gdb) frame 1
#1  0x0004474c in Heavy_bela::Heavy_bela (this=0xa2e58, sampleRate=44100, 
    poolKb=10, inQueueKb=2, outQueueKb=0)
    at /root/Bela/projects/reverb/Heavy_bela.cpp:66
66	  numBytes += sLine_init(&sLine_Eco7brLC);
(gdb) frame 2
#2  0x000446c8 in hv_bela_new_with_options (sampleRate=44100, poolKb=10, 
    inQueueKb=2, outQueueKb=0) at /root/Bela/projects/reverb/Heavy_bela.cpp:50
50	    return new Heavy_bela(sampleRate, poolKb, inQueueKb, outQueueKb);
(gdb) frame 3
#3  0x00067c14 in setup (context=0x8a9f0 <gContext>, userData=0x0)
    at /root/Bela/projects/reverb/render.cpp:399
399		gHeavyContext = hv_bela_new_with_options(context->audioSampleRate, 10, 2, 0);
(gdb) frame 4
#4  0x00026b3c in Bela_initAudio ()
(gdb) frame 5
#5  0x00019312 in main ()
(gdb)

giuliomoro · May 3, 2019

well, so it's indeed an alignment issue as in the earlier issue. First off, I should have asked you to compile with CPPFLAGS="-O0 -g" CFLAGS="-O0 -g" (as it affects both C++ and C files), then you'd be able to see:

Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
print o-[New Thread 0xb693c450 (LWP 28552)]
__hv_noteout: 0xd1d4ac2
__hv_ctlout: 0xe5e2a040
__hv_pgmout: 0x8753e39e
__hv_touchout: 0x476d4387
__hv_polytouchout: 0xd5aca9d1
__hv_bendout: 0xe8458013
__hv_midiout: 0x6511de55
: 0
>
Thread 1 "reverbtest" received signal SIGBUS, Bus error.
sLine_init (o=0xa6d48) at /root/Bela/projects/reverbtest/HvSignalLine.c:32
32	  o->x = vdupq_n_f32(0.0f);

Then you'd inspect the address of o->x:

(gdb) print &o->x
$3 = (float32x4_t *) 0xa6d58
(gdb)

and you see that it is a float32x4_t which should be aligned to 16 bytes, but is actually aligned to 8 bytes.

Turns out the problem is the whole Heavy_bela object should be aligned to 16 bytes, but it is not. I will have to dig deeper into this.

Llokki · May 3, 2019

giuliomoro I will have to dig deeper into this.

i see, so would one option be to change the heavy generated c files before compiling? or is there no way around the bug in clang?

thanks for looking into this and i guess i will stay with gcc for the moment.

giuliomoro · May 3, 2019

digging deeper, it seems it really is an issue with the clang.

This simple test program

#include <arm_neon.h>
#include <stdio.h>
#include <assert.h>

class MyClass {
public:
        float32x4_t c;
};

int main()
{
        printf("alignof(MyClass): %d\n", alignof(MyClass));
        printf("alignof(float32x4_t): %d\n", alignof(float32x4_t));
        for(int n = 0; n < 100; ++n)
        {
                auto ptr = new MyClass;
                printf("ptr: %p\n", ptr);
                printf("c: %p\n", &ptr->c);
                assert(((unsigned int)ptr & (alignof(MyClass) - 1)) == 0);
        }
        return 0;
}

fails with clang. It work with gcc, but then gcc claims that alignof(float32x4_t) is 8 instead of 16, which I don't understand ...

So, a current workaround would be to override operator new in the Heavy_bela class adding to the class Heavy_bela class declaration in Heavy_bela.hpp:

#include <new>
....
class Heavy_bela {
...
  void* operator new(size_t sz) {
    auto ptr = aligned_alloc(16, sz);
    if(!ptr)
    {
     std::bad_alloc exception;
     throw exception;
    }
    return ptr;
  }
...
}

however, the issue is fairly worrying and I need to find out if this is related to the version of clang, and if there is a more up-to-date one we can use instead.

If this turns out to be needed, we could add the change to the hvcc template files.

AndyCap · May 3, 2019

While I was looking into the performance align I noticed there is a Clang command line flag to set the default new() align, might be worth looking at that?

giuliomoro · May 3, 2019

What I found is an ancient discussion of gcc devs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=15795 . They mention that according to the c++ standard, the alignment of the pointer returned by operator new is undefined. As such, the behaviout observed with clang is the expected behaviour, however this makes all c++ code using the default operator new for classes with intrinsics unusable on Linux, where the underlying call to malloc() returns values aligned to 8 byte. On Mac (and I shall assume iOS), malloc() always returns a pointer aligned to 16 bytes, making this not an issue for many intrinsics.

So yes, please, if you found a suitable flag for clang, tell us!

AndyCap · May 3, 2019

Try:

-faligned-new=16

Llokki · May 4, 2019

AndyCap will have a go at this tomorrow, thanks

giuliomoro · May 4, 2019

looks like that option is not supported on the clang / gcc on the board. It was probably introduced more recently.

AndyCap · May 4, 2019

Ah, what version of clang is in the board?

giuliomoro · May 4, 2019

It's 3.9 on the board. clang 6.0 seems to have it, and you can install it with apt-get from stretch backport (though I haven't tried: I just downloaded if from llvm.org).

Haven't managed to make it produce the desired code yet, though.

giuliomoro · May 4, 2019

For whatever reason, on clang-3.9, alignof(float32x4_t) is 16 bytes, while on clang-6 (and on gcc-6.3), it is 8 bytes. Especially the clang-3.9 vs 6 is particularly weird, because their respective arm_neon.h includes are very similar ....

AndyCap · May 5, 2019

I'm a bit confused!

I though the original issue was that 3.9 wasn't aligning to 16?

Llokki · May 5, 2019

this is for android (arm) but seems to talk about the clang problem:

https://github.com/axboe/fio/issues/356

giuliomoro · May 6, 2019

AndyCap I though the original issue was that 3.9 wasn't aligning to 16?

The original issue is that on clang-3.9, alignof(float32x4_t) is 16, and it (probably) generates assembly code that assumes the memory to be aligned. If the memory is not effectively aligned to a 16 bytes boundaries, you get a SIGBUS.

However, when allocating with operator new an object of a class that contains one or more float32x4_t, there is no guarantee that these will effectively be allocated with the appropriate alignment. The underlying issue is that according to the C++ standard, the alignment of a pointer returned by operator new is undefined. Hence the workaround above with a custom allocator. It seems that C++17 offers some more clever support for aligned heap allocation, but I didn't get into the details of that.

What is surprising is that while alignof(float32x4_t) is 16 bytes on clang-3.9, it is actually 8 bytes on gcc-6.3 and clang-6.0. I guess that this could explain the both:
- the performance penalty for gcc-6.3 vs clang-3.9 when using intrinsics
- the lack of SIGBUS on gcc-6.3: the generated assembly does not make assumptions about alignment that will then be neglected at runtime.

Seeing how this is alignof(float32x4_t) has become 8 bytes in clang-6.0, I am afraid this may also come with a performance penalty (but I haven't verified it yet). However, I am wondering where that difference comes from, as the arm_neon.h files for the two versions of clang provide identical definition for all the neon vector types.

Incidentally, given how the change in alignof can also change the size of structs and classes, this suggest that header files for libraries should abstract away such classes and structs, so that the library will work fine even if the compiler used to compile it is different from the one used to build the user code that uses it.

Llokki · May 6, 2019

giuliomoro

so where exactly do i put this in the file? in the public part of the class? sorry if this is obvious....

giuliomoro · May 6, 2019