From 60ad7f80ccf159b7fe3b1f1cbbb7e39795838394 Mon Sep 17 00:00:00 2001 From: Mooneer Salem Date: Sun, 1 Jan 2023 00:41:39 -0800 Subject: [PATCH 1/4] M1 Mac is fast enough to require USE_MUTEX for test_fifo to consistently pass. --- src/codec2_fifo.c | 41 ++++++++++++++++++++++------------------- unittest/tfifo.c | 3 ++- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/codec2_fifo.c b/src/codec2_fifo.c index a2d3c3099..b93c7c346 100644 --- a/src/codec2_fifo.c +++ b/src/codec2_fifo.c @@ -30,8 +30,9 @@ */ #include -#include #include +#include +#include #include "codec2_fifo.h" struct FIFO { @@ -70,7 +71,6 @@ void codec2_fifo_destroy(struct FIFO *fifo) { } int codec2_fifo_write(struct FIFO *fifo, short data[], int n) { - int i; short *pdata; short *pin = fifo->pin; @@ -82,15 +82,17 @@ int codec2_fifo_write(struct FIFO *fifo, short data[], int n) { } else { - /* This could be made more efficient with block copies - using memcpy */ - pdata = data; - for(i=0; ibuf + fifo->nshort)) - pin = fifo->buf; - } + if ((pin + n) >= (fifo->buf + fifo->nshort)) + { + int firstSamples = fifo->buf + fifo->nshort - pin; + memcpy(pin, pdata, firstSamples * sizeof(short)); + n -= firstSamples; + pin = fifo->buf; + pdata += firstSamples; + } + memcpy(pin, pdata, n * sizeof(short)); + pin += n; fifo->pin = pin; } @@ -99,7 +101,6 @@ int codec2_fifo_write(struct FIFO *fifo, short data[], int n) { int codec2_fifo_read(struct FIFO *fifo, short data[], int n) { - int i; short *pdata; short *pout = fifo->pout; @@ -111,15 +112,17 @@ int codec2_fifo_read(struct FIFO *fifo, short data[], int n) } else { - /* This could be made more efficient with block copies - using memcpy */ - pdata = data; - for(i=0; ibuf + fifo->nshort)) - pout = fifo->buf; - } + if ((pout + n) >= (fifo->buf + fifo->nshort)) + { + int firstSamples = fifo->buf + fifo->nshort - pout; + memcpy(pdata, pout, firstSamples * sizeof(short)); + n -= firstSamples; + pout = fifo->buf; + pdata += firstSamples; + } + memcpy(pdata, pout, n * sizeof(short)); + pout += n; fifo->pout = pout; } diff --git a/unittest/tfifo.c b/unittest/tfifo.c index 0987db67b..1192299ff 100644 --- a/unittest/tfifo.c +++ b/unittest/tfifo.c @@ -9,6 +9,7 @@ #include #include #include +#include #include "codec2_fifo.h" #define FIFO_SZ 1024 @@ -25,7 +26,7 @@ void *writer_thread(void *data); pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; #define USE_THREADS -//#define USE_MUTEX +#define USE_MUTEX int main() { pthread_t awriter_thread; From 6b6fe2f11b973ae69631c6f7a8110b15930ae066 Mon Sep 17 00:00:00 2001 From: Mooneer Salem Date: Tue, 10 Jan 2023 01:23:17 -0800 Subject: [PATCH 2/4] 100% pass on M1 Mac (10/10 runs) using stdatomic.h. --- src/codec2_fifo.c | 60 +++++++++++++++++++++++++++++++++++++---------- unittest/tfifo.c | 2 +- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/codec2_fifo.c b/src/codec2_fifo.c index b93c7c346..63872025e 100644 --- a/src/codec2_fifo.c +++ b/src/codec2_fifo.c @@ -33,12 +33,22 @@ #include #include #include + +#ifndef __STDC_NO_ATOMICS__ +#include +#endif /* !__STDC_NO_ATOMICS__ */ + #include "codec2_fifo.h" struct FIFO { short *buf; +#ifdef __STDC_NO_ATOMICS__ short *pin; short *pout; +#else + atomic_intptr_t pin; + atomic_intptr_t pout; +#endif /* __STDC_NO_ATOMICS__ */ int nshort; }; @@ -57,8 +67,13 @@ struct FIFO *codec2_fifo_create_buf(int nshort, short* buf) { assert(fifo != NULL); fifo->buf = buf; +#ifdef __STDC_NO_ATOMICS__ fifo->pin = fifo->buf; fifo->pout = fifo->buf; +#else + atomic_store(&fifo->pin, (intptr_t)fifo->buf); + atomic_store(&fifo->pout, (intptr_t)fifo->buf); +#endif /* __STDC_NO_ATOMICS__ */ fifo->nshort = nshort; return fifo; @@ -71,16 +86,20 @@ void codec2_fifo_destroy(struct FIFO *fifo) { } int codec2_fifo_write(struct FIFO *fifo, short data[], int n) { - short *pdata; - short *pin = fifo->pin; - assert(fifo != NULL); assert(data != NULL); - if (n > codec2_fifo_free(fifo)) { - return -1; + int free = codec2_fifo_free(fifo); + if (n > free) { + return -1; } else { + short *pdata; +#ifdef __STDC_NO_ATOMICS__ + short *pin = fifo->pin; +#else + short *pin = (short*)atomic_load(&fifo->pin); +#endif /* __STDC_NO_ATOMICS__ */ pdata = data; if ((pin + n) >= (fifo->buf + fifo->nshort)) @@ -93,7 +112,11 @@ int codec2_fifo_write(struct FIFO *fifo, short data[], int n) { } memcpy(pin, pdata, n * sizeof(short)); pin += n; - fifo->pin = pin; +#ifdef __STDC_NO_ATOMICS__ + fifo->pin = pin; +#else + atomic_store(&fifo->pin, (intptr_t)pin); +#endif /* __STDC_NO_ATOMICS__ */ } return 0; @@ -101,16 +124,20 @@ int codec2_fifo_write(struct FIFO *fifo, short data[], int n) { int codec2_fifo_read(struct FIFO *fifo, short data[], int n) { - short *pdata; - short *pout = fifo->pout; - assert(fifo != NULL); assert(data != NULL); - if (n > codec2_fifo_used(fifo)) { - return -1; + int used = codec2_fifo_used(fifo); + if (n > used) { + return -1; } else { + short *pdata; +#ifdef __STDC_NO_ATOMICS__ + short *pout = fifo->pout; +#else + short *pout = (short*)atomic_load(&fifo->pout); +#endif /* __STDC_NO_ATOMICS__ */ pdata = data; if ((pout + n) >= (fifo->buf + fifo->nshort)) @@ -123,7 +150,11 @@ int codec2_fifo_read(struct FIFO *fifo, short data[], int n) } memcpy(pdata, pout, n * sizeof(short)); pout += n; - fifo->pout = pout; +#ifdef __STDC_NO_ATOMICS__ + fifo->pout = pout; +#else + atomic_store(&fifo->pout, (intptr_t)pout); +#endif /* __STDC_NO_ATOMICS__ */ } return 0; @@ -131,8 +162,13 @@ int codec2_fifo_read(struct FIFO *fifo, short data[], int n) int codec2_fifo_used(const struct FIFO * const fifo) { +#ifdef __STDC_NO_ATOMICS__ short *pin = fifo->pin; short *pout = fifo->pout; +#else + short *pin = (short*)atomic_load(&fifo->pin); + short *pout = (short*)atomic_load(&fifo->pout); +#endif /* __STDC_NO_ATOMICS__ */ unsigned int used; assert(fifo != NULL); diff --git a/unittest/tfifo.c b/unittest/tfifo.c index 1192299ff..c31cd6cf1 100644 --- a/unittest/tfifo.c +++ b/unittest/tfifo.c @@ -26,7 +26,7 @@ void *writer_thread(void *data); pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; #define USE_THREADS -#define USE_MUTEX +//#define USE_MUTEX int main() { pthread_t awriter_thread; From 9dbec98b297914dadae9b0620b397453f2d53818 Mon Sep 17 00:00:00 2001 From: Mooneer Salem Date: Tue, 10 Jan 2023 01:26:24 -0800 Subject: [PATCH 3/4] Fix Linux build error. --- src/codec2_fifo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/codec2_fifo.c b/src/codec2_fifo.c index 63872025e..6b09761d7 100644 --- a/src/codec2_fifo.c +++ b/src/codec2_fifo.c @@ -35,6 +35,7 @@ #include #ifndef __STDC_NO_ATOMICS__ +#include #include #endif /* !__STDC_NO_ATOMICS__ */ From 59d927e0aea59ee1cbefb4a96940bc3f70cde048 Mon Sep 17 00:00:00 2001 From: Mooneer Salem Date: Tue, 10 Jan 2023 01:53:12 -0800 Subject: [PATCH 4/4] We should atomic load once and atomic store once per operation. --- src/codec2_fifo.c | 84 +++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 32 deletions(-) diff --git a/src/codec2_fifo.c b/src/codec2_fifo.c index 6b09761d7..fc16783be 100644 --- a/src/codec2_fifo.c +++ b/src/codec2_fifo.c @@ -86,23 +86,44 @@ void codec2_fifo_destroy(struct FIFO *fifo) { free(fifo); } +static int codec2_fifo_used_internal(short* pin, short* pout, int nshort) +{ + int used = 0; + + if (pin >= pout) + used = pin - pout; + else + used = nshort + (unsigned int)(pin - pout); + + return used; +} + +static int codec2_fifo_free_internal(short* pin, short* pout, int nshort) +{ + // available storage is one less than nshort as prd == pwr + // is reserved for empty rather than full + + return nshort - codec2_fifo_used_internal(pin, pout, nshort) - 1; +} + int codec2_fifo_write(struct FIFO *fifo, short data[], int n) { assert(fifo != NULL); assert(data != NULL); - int free = codec2_fifo_free(fifo); +#ifdef __STDC_NO_ATOMICS__ + short *pin = fifo->pin; + short *pin = fifo->pout; +#else + short *pin = (short*)atomic_load(&fifo->pin); + short *pout = (short*)atomic_load(&fifo->pout); +#endif /* __STDC_NO_ATOMICS__ */ + + int free = codec2_fifo_free_internal(pin, pout, fifo->nshort); if (n > free) { return -1; } else { - short *pdata; -#ifdef __STDC_NO_ATOMICS__ - short *pin = fifo->pin; -#else - short *pin = (short*)atomic_load(&fifo->pin); -#endif /* __STDC_NO_ATOMICS__ */ - - pdata = data; + short *pdata = data; if ((pin + n) >= (fifo->buf + fifo->nshort)) { int firstSamples = fifo->buf + fifo->nshort - pin; @@ -116,7 +137,7 @@ int codec2_fifo_write(struct FIFO *fifo, short data[], int n) { #ifdef __STDC_NO_ATOMICS__ fifo->pin = pin; #else - atomic_store(&fifo->pin, (intptr_t)pin); + atomic_store(&fifo->pin, (intptr_t)pin); #endif /* __STDC_NO_ATOMICS__ */ } @@ -127,20 +148,21 @@ int codec2_fifo_read(struct FIFO *fifo, short data[], int n) { assert(fifo != NULL); assert(data != NULL); + +#ifdef __STDC_NO_ATOMICS__ + short *pin = fifo->pin; + short *pin = fifo->pout; +#else + short *pin = (short*)atomic_load(&fifo->pin); + short *pout = (short*)atomic_load(&fifo->pout); +#endif /* __STDC_NO_ATOMICS__ */ - int used = codec2_fifo_used(fifo); + int used = codec2_fifo_used_internal(pin, pout, fifo->nshort); if (n > used) { return -1; } else { - short *pdata; -#ifdef __STDC_NO_ATOMICS__ - short *pout = fifo->pout; -#else - short *pout = (short*)atomic_load(&fifo->pout); -#endif /* __STDC_NO_ATOMICS__ */ - - pdata = data; + short *pdata = data; if ((pout + n) >= (fifo->buf + fifo->nshort)) { int firstSamples = fifo->buf + fifo->nshort - pout; @@ -170,21 +192,19 @@ int codec2_fifo_used(const struct FIFO * const fifo) short *pin = (short*)atomic_load(&fifo->pin); short *pout = (short*)atomic_load(&fifo->pout); #endif /* __STDC_NO_ATOMICS__ */ - unsigned int used; - - assert(fifo != NULL); - if (pin >= pout) - used = pin - pout; - else - used = fifo->nshort + (unsigned int)(pin - pout); - - return used; + + return codec2_fifo_used_internal(pin, pout, fifo->nshort); } int codec2_fifo_free(const struct FIFO * const fifo) { - // available storage is one less than nshort as prd == pwr - // is reserved for empty rather than full - - return fifo->nshort - codec2_fifo_used(fifo) - 1; +#ifdef __STDC_NO_ATOMICS__ + short *pin = fifo->pin; + short *pout = fifo->pout; +#else + short *pin = (short*)atomic_load(&fifo->pin); + short *pout = (short*)atomic_load(&fifo->pout); +#endif /* __STDC_NO_ATOMICS__ */ + + return codec2_fifo_free_internal(pin, pout, fifo->nshort); }