lf-lang / lingua-franca

Intuitive concurrent programming in any language
https://www.lf-lang.org
Other
220 stars 57 forks source link

Support for Patmos platform #2315

Open EhsanKhodadad opened 3 weeks ago

schoeberl commented 2 weeks ago

In general, as it is mostly Patmos-specific changes, it looks fine. However, 2 points: (1) should a change in core/threaded/scheduler_static.c really be part of this commit? @lsk567 needs to approve those changes. (2) is test/C the best place for Patmos-specific README and Makefile?

EhsanKhodadad commented 2 weeks ago

In general, as it is mostly Patmos-specific changes, it looks fine. However, 2 points: (1) should a change in core/threaded/scheduler_static.c really be part of this commit? @lsk567 needs to approve those changes. (2) is test/C the best place for Patmos-specific README and Makefile?

About core/threaded/scheduler_static.c, we only needed to comment out #include "semaphore.h" in line 55 to get rid of a compile error; other changes are made by the formatter. Regarding the README and Makefile places, since we already had README files in both root and test directories, I decided to put them in the test/C directory because it was not only near the root but also near C/src/static/Patmos. Nevertheless, I will move them if you have other opinions or suggestions.

lsk567 commented 2 weeks ago

@EhsanKhodadad Thanks for working on the Patmos support and get them to work with the static scheduler. However, I don't think these files belong in this lingua-franca repository. They seem to belong to the reactor-c repository.

Suggestions for a proper integration:

But given that this branch for single-threaded execution is not a stable solution (code from the threaded folder is leaking into single-threaded code), I suggest we not merge static-schedule-single-threaded into static-schedule or master. All the changes can park on this branch or a separate repo for this upcoming paper.

Then, we can work on the proper Patmos integration when we can print static schedules into a main() function. Therefore, I suggest we come back to this PR later.