search

espressif / esp-dsp

DSP library for ESP-IDF
Apache License 2.0
442 stars 87 forks source link

Memory alignment issues (DSP-80) #46

Open Barabas5532 opened 2 years ago

Barabas5532 commented 2 years ago

There are some places in this library where the user must align memory passed to functions correctly. These are not documented anywhere I can find.

E.G. The dsps_fft4r_fc32_ansi_ function casts the float *data parameter to the following struct:

typedef union fc32_u
{
    struct
    {
        float re;
        float im;
    };
    uint64_t data;
}fc32_t;

The uint64_t member of this union requires 8 bytes alignment. Actually, this member is unused, so it would be best to remove it, and make the default 4 byte alignment work. Why doesn't this library use the C99 complex type feature?

I've also noticed that 16 byte alignment was added to a lot of the example apps with the support for ESP32S3 chip in https://github.com/espressif/esp-dsp/commit/108493c53f8d0de80be5494a74710e6492ba7443. Is this alignment only required for implementations using assembly code for that chip?

https://github.com/espressif/esp-dsp/blob/3175e829b3af66dcd68e98a95a16b0868f047ee0/modules/common/include/dsp_tests.h#L34

The tests disable alignment on esp-idf version <4.3.0 by defining memalign as malloc and ignoring the alignment parameter. Would this not cause issues when loading memory from an unaligned address?

igrr commented 2 years ago

The tests disable alignment on esp-idf version <4.3.0 by defining memalign as malloc and ignoring the alignment parameter.

I think the reason is that older versions of IDF didn't have memalign implemented. And the 16 byte alignment is necessary for esp32-s3, which is supported only since IDF 4.4.

Good point about documenting the alignment requirements, we'll update the docs!