Skip to content

multi-thread in the Patmos Platform #582

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.

Sign up for GitHub

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

Open
EhsanKhodadad wants to merge 2 commits into
base: main
Choose a base branch
from
Open

multi-thread in the Patmos Platform #582

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3467fa5
multi-thread in the Patmos Platform
EhsanKhodadad Apr 29, 2026
d2daa38
reviews were addressed
EhsanKhodadad Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions low_level_platform/api/platform/lf_patmos_support.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,11 @@
#define PRINTF_TIME "%" PRId64
#define PRINTF_MICROSTEP "%" PRIu32
#define PRINTF_TAG "(%" PRId64 ", %" PRIu32 ")"
#if !defined(LF_SINGLE_THREADED)
#include <pthread.h>
typedef pthread_t lf_thread_t;
typedef void* lf_mutex_t;
typedef void* lf_cond_t;
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new threaded Patmos typedefs (lf_mutex_t / lf_cond_t as void*) don’t match how the core platform API expects these types to work (condition variables need to carry a mutex pointer; pthread-based platforms use concrete types like pthread_mutex_t and a struct lf_cond_t { lf_mutex_t* mutex; pthread_cond_t condition; }). Consider aligning Patmos with lf_POSIX_threads_support.h so lf_cond_wait/_lf_cond_timedwait can be correctly implemented and so the runtime can embed these objects directly without heap allocation.

Suggested change
typedef void* lf_mutex_t;
typedef void* lf_cond_t;
typedef pthread_mutex_t lf_mutex_t;
typedef struct {
lf_mutex_t* mutex;
pthread_cond_t condition;
} lf_cond_t;

Copilot uses AI. Check for mistakes.
#endif

#endif // LF_PATMOS_SUPPORT_H
102 changes: 101 additions & 1 deletion low_level_platform/impl/src/lf_patmos_support.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,106 @@ int _lf_single_threaded_notify_of_event() {
_lf_async_event = true;
return 0;
}
#endif // LF_SINGLE_THREADED
#else // LF_SINGLE_THREADED

int lf_disable_interrupts_nested() {
if (_lf_num_nested_critical_sections++ == 0) {
intr_disable();
}
return 0;
Comment on lines +131 to +136
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the threaded (#else) branch, _lf_num_nested_critical_sections is a single global counter shared by all threads/cores. With multiple threads, this can prevent intr_disable() from being called on a core that enters its first critical section while another core is already in one (counter != 0), leaving interrupts enabled on that core. Make the nesting counter per-core/per-thread (similar to FlexPRET’s critical_section_num_nested[hartid]) or otherwise ensure interrupt masking is correctly applied independently on each core.

Copilot uses AI. Check for mistakes.
}

int lf_enable_interrupts_nested() {
if (_lf_num_nested_critical_sections <= 0) {
return 1;
}

if (--_lf_num_nested_critical_sections == 0) {
intr_enable();
}
return 0;
}

int lf_available_cores() { return (int)get_cpucnt(); }

lf_thread_t lf_thread_self() {
lf_thread_t self = {0};
self.cpuid = (int)get_cpuid();
return self;
Comment on lines +154 to +156
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lf_thread_t is defined as pthread_t in lf_patmos_support.h, but here lf_thread_self() treats it as a struct with a cpuid field (lf_thread_t self = {0}; self.cpuid = ...). This will not compile and also conflicts with how lf_thread_create/join cast to pthread_t. Make lf_thread_t consistent across header and implementation (e.g., use pthread_t everywhere and return pthread_self(), or define a Patmos-specific struct type in the header and adjust create/join accordingly).

Suggested change
lf_thread_t self = {0};
self.cpuid = (int)get_cpuid();
return self;
return pthread_self();

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@EhsanKhodadad EhsanKhodadad Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggested change was not compatible with Patmos implementation

}

int lf_thread_create(lf_thread_t* thread, void* (*lf_thread)(void*), void* arguments) {
assert(thread != NULL);
return pthread_create((pthread_t*)thread, NULL, lf_thread, arguments);
}

int lf_thread_join(lf_thread_t thread, void** thread_return) {
return pthread_join((pthread_t)thread, thread_return);
}

int lf_thread_id() { return (int)get_cpuid(); }

void initialize_lf_thread_id() {}

Comment on lines +168 to +171
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lf_thread_id() / initialize_lf_thread_id() are also provided by low_level_platform/impl/src/lf_platform_util.c for all non-Zephyr, non-single-threaded builds. Adding Patmos-specific definitions here will cause duplicate symbol/linker errors in threaded Patmos builds. Either remove these definitions and rely on the shared implementation, or change the build/guards so only one implementation is compiled.

Suggested change
int lf_thread_id() { return (int)get_cpuid(); }
void initialize_lf_thread_id() {}

Copilot uses AI. Check for mistakes.
int lf_mutex_init(lf_mutex_t* mutex) {
assert(mutex != NULL);
*mutex = (lf_mutex_t)malloc(sizeof(pthread_mutex_t));
if (*mutex == NULL) return -1;
return pthread_mutex_init((pthread_mutex_t*)*mutex, NULL);
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lf_mutex_init() initializes a default (non-recursive) pthread_mutex_t. The runtime’s POSIX implementation explicitly uses a recursive mutex to tolerate re-entrant locking in the core (see lf_POSIX_threads_support.c). If Patmos is using pthreads, this should match by setting the mutex type to recursive; otherwise threaded Patmos can deadlock when the same thread locks the same mutex twice.

Suggested change
assert(mutex != NULL);
*mutex = (lf_mutex_t)malloc(sizeof(pthread_mutex_t));
if (*mutex == NULL) return -1;
return pthread_mutex_init((pthread_mutex_t*)*mutex, NULL);
int result;
pthread_mutexattr_t attr;
assert(mutex != NULL);
*mutex = (lf_mutex_t)malloc(sizeof(pthread_mutex_t));
if (*mutex == NULL) return -1;
result = pthread_mutexattr_init(&attr);
if (result != 0) {
free(*mutex);
*mutex = NULL;
return result;
}
result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
if (result != 0) {
pthread_mutexattr_destroy(&attr);
free(*mutex);
*mutex = NULL;
return result;
}
result = pthread_mutex_init((pthread_mutex_t*)*mutex, &attr);
pthread_mutexattr_destroy(&attr);
if (result != 0) {
free(*mutex);
*mutex = NULL;
}
return result;

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

malloc() is used here, but lf_patmos_support.c does not include <stdlib.h>. This will fail to compile on toolchains that don’t implicitly declare malloc. Add the proper include (or avoid heap allocation by storing pthread_mutex_t inline as other pthread-based platforms do).

Copilot uses AI. Check for mistakes.
}

int lf_mutex_lock(lf_mutex_t* mutex) {
assert(mutex != NULL && *mutex != NULL);
return pthread_mutex_lock((pthread_mutex_t*)*mutex);
}

int lf_mutex_unlock(lf_mutex_t* mutex) {
assert(mutex != NULL && *mutex != NULL);
return pthread_mutex_unlock((pthread_mutex_t*)*mutex);
}

int lf_cond_init(lf_cond_t* cond, lf_mutex_t* mutex) {
assert(cond != NULL);
*cond = (lf_cond_t)malloc(sizeof(pthread_cond_t));
if (*cond == NULL) return -1;
return pthread_cond_init((pthread_cond_t*)*cond, NULL);
}
Comment on lines +204 to +209
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lf_cond_init() accepts an associated mutex but does not store or use it, which makes it impossible to implement lf_cond_wait() correctly. The core API assumes the condition variable knows its associated mutex (see low_level_platform.h docs and the POSIX implementation). Update lf_cond_t to include a mutex pointer (or otherwise bind the mutex here) and use it in lf_cond_wait/_lf_cond_timedwait.

Copilot uses AI. Check for mistakes.

int lf_cond_signal(lf_cond_t* cond) {
assert(cond != NULL && *cond != NULL);
return pthread_cond_signal((pthread_cond_t*)*cond);
}

int lf_cond_broadcast(lf_cond_t* cond) {
assert(cond != NULL && *cond != NULL);
return pthread_cond_broadcast((pthread_cond_t*)*cond);
}

int lf_cond_wait(lf_cond_t* cond) {
assert(cond != NULL && *cond != NULL);
// Note: This requires that the caller has locked the associated mutex.
// For bare-metal Patmos, we assume the calling code maintains this invariant.
// In practice, cond_wait should be used with a mutex like:
// pthread_mutex_lock(&mutex);
// while (!condition) pthread_cond_wait(&cond, &mutex);
// pthread_mutex_unlock(&mutex);
// Since we don't track the associated mutex here, the caller must manage it.
return -1; // Not directly supported; use architecture-specific approach
}

int _lf_cond_timedwait(lf_cond_t* cond, instant_t wakeup_time) {
assert(cond != NULL && *cond != NULL);
instant_t now;
_lf_clock_gettime(&now);

if (now >= wakeup_time) {
return LF_TIMEOUT;
}

// Patmos does not provide a native blocking condition-variable timeout here.
return LF_TIMEOUT; // Treat expired or unsupported waits as timeout on Patmos.
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lf_cond_wait() currently always returns -1 (i.e., never blocks). This breaks core runtime code that relies on condition variables for correctness (e.g., semaphores and environment startup barriers call lf_cond_wait() in loops). Implement this using pthread_cond_wait() (and ensure lf_cond_t carries its associated mutex) or provide an equivalent blocking primitive on Patmos.

Suggested change
int lf_cond_init(lf_cond_t* cond, lf_mutex_t* mutex) {
assert(cond != NULL);
*cond = (lf_cond_t)malloc(sizeof(pthread_cond_t));
if (*cond == NULL) return -1;
return pthread_cond_init((pthread_cond_t*)*cond, NULL);
}
int lf_cond_signal(lf_cond_t* cond) {
assert(cond != NULL && *cond != NULL);
return pthread_cond_signal((pthread_cond_t*)*cond);
}
int lf_cond_broadcast(lf_cond_t* cond) {
assert(cond != NULL && *cond != NULL);
return pthread_cond_broadcast((pthread_cond_t*)*cond);
}
int lf_cond_wait(lf_cond_t* cond) {
assert(cond != NULL && *cond != NULL);
// Note: This requires that the caller has locked the associated mutex.
// For bare-metal Patmos, we assume the calling code maintains this invariant.
// In practice, cond_wait should be used with a mutex like:
// pthread_mutex_lock(&mutex);
// while (!condition) pthread_cond_wait(&cond, &mutex);
// pthread_mutex_unlock(&mutex);
// Since we don't track the associated mutex here, the caller must manage it.
return -1; // Not directly supported; use architecture-specific approach
}
int _lf_cond_timedwait(lf_cond_t* cond, instant_t wakeup_time) {
assert(cond != NULL && *cond != NULL);
instant_t now;
_lf_clock_gettime(&now);
if (now >= wakeup_time) {
return LF_TIMEOUT;
}
// Patmos does not provide a native blocking condition-variable timeout here.
return LF_TIMEOUT; // Treat expired or unsupported waits as timeout on Patmos.
typedef struct {
pthread_cond_t cond;
lf_mutex_t* mutex;
} lf_patmos_cond_t;
int lf_cond_init(lf_cond_t* cond, lf_mutex_t* mutex) {
lf_patmos_cond_t* patmos_cond;
assert(cond != NULL);
assert(mutex != NULL);
patmos_cond = (lf_patmos_cond_t*)malloc(sizeof(lf_patmos_cond_t));
if (patmos_cond == NULL) return -1;
patmos_cond->mutex = mutex;
if (pthread_cond_init(&patmos_cond->cond, NULL) != 0) {
free(patmos_cond);
return -1;
}
*cond = (lf_cond_t)patmos_cond;
return 0;
}
int lf_cond_signal(lf_cond_t* cond) {
lf_patmos_cond_t* patmos_cond;
assert(cond != NULL && *cond != NULL);
patmos_cond = (lf_patmos_cond_t*)*cond;
return pthread_cond_signal(&patmos_cond->cond);
}
int lf_cond_broadcast(lf_cond_t* cond) {
lf_patmos_cond_t* patmos_cond;
assert(cond != NULL && *cond != NULL);
patmos_cond = (lf_patmos_cond_t*)*cond;
return pthread_cond_broadcast(&patmos_cond->cond);
}
int lf_cond_wait(lf_cond_t* cond) {
lf_patmos_cond_t* patmos_cond;
assert(cond != NULL && *cond != NULL);
patmos_cond = (lf_patmos_cond_t*)*cond;
assert(patmos_cond->mutex != NULL && *patmos_cond->mutex != NULL);
return pthread_cond_wait(&patmos_cond->cond, (pthread_mutex_t*)*patmos_cond->mutex);
}
int _lf_cond_timedwait(lf_cond_t* cond, instant_t wakeup_time) {
lf_patmos_cond_t* patmos_cond;
instant_t now;
struct timespec ts;
int rc;
assert(cond != NULL && *cond != NULL);
patmos_cond = (lf_patmos_cond_t*)*cond;
assert(patmos_cond->mutex != NULL && *patmos_cond->mutex != NULL);
_lf_clock_gettime(&now);
if (now >= wakeup_time) {
return LF_TIMEOUT;
}
ts.tv_sec = wakeup_time / 1000000000LL;
ts.tv_nsec = wakeup_time % 1000000000LL;
rc = pthread_cond_timedwait(&patmos_cond->cond, (pthread_mutex_t*)*patmos_cond->mutex, &ts);
if (rc == ETIMEDOUT) {
return LF_TIMEOUT;
}
return rc;

Copilot uses AI. Check for mistakes.
}
Comment on lines +227 to +249
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_lf_cond_timedwait() always returns LF_TIMEOUT without waiting when now < wakeup_time. Callers (e.g., watchdog and clock waits) expect this to block until either signaled or the timeout expires; returning immediately will cause busy looping and incorrect timing behavior. Implement a real timed wait (e.g., via pthread_cond_timedwait() with an absolute timespec) or an equivalent Patmos-specific mechanism.

Copilot uses AI. Check for mistakes.

#endif

#endif // PLATFORM_PATMOS
Loading