Support for FlexPRET platform

[magnmaeh]

This PR adds support for the FlexPRET platform. In addition, it adds an Option<T> on the PlatformOptions type so we know whether a specific key/value pair was set by the user or just had its default value.

Depends on lf-lang/reactor-c#412

[magnmaeh]

@lhstrh / @erlingrj I'm having an issue with the baudrate target property. Specifically this line, which sets the default baudrate for a platform if the user does not specify any:

lingua-franca/core/src/main/java/org/lflang/target/property/PlatformProperty.java

Line 43 in f840daa

var baudRate = 9600;

. The reason for this is: "9600 is a standard amongst systems like Arduino, so it's the default value."

When there is a default baudrate, I'm not sure if I'll be able to determine whether the user tried to set the property or if it is just a default value. (If there is a way to do this, please let me know; I haven't programmed in Java until now, so I'm struggling to understand everything going on.) What I want to do for FlexPRET is not allow the user to set it, because this is something the FlexPRET SDK controls entirely.

Maybe it would be possible to provide different default values depending on what platform is used? Or just set the default value of baudrate to 0 and have Arduino set it to 9600 somewhere else.

[lhstrh]

@lhstrh / @erlingrj I'm having an issue with the baudrate target property. Specifically this line, which sets the default baudrate for a platform if the user does not specify any:

lingua-franca/core/src/main/java/org/lflang/target/property/PlatformProperty.java

Line 43 in f840daa

var baudRate = 9600;

. The reason for this is: "9600 is a standard amongst systems like Arduino, so it's the default value."

When there is a default baudrate, I'm not sure if I'll be able to determine whether the user tried to set the property or if it is just a default value. (If there is a way to do this, please let me know; I haven't programmed in Java until now, so I'm struggling to understand everything going on.) What I want to do for FlexPRET is not allow the user to set it, because this is something the FlexPRET SDK controls entirely.

Maybe it would be possible to provide different default values depending on what platform is used? Or just set the default value of baudrate to 0 and have Arduino set it to 9600 somewhere else.

The way it's currently set up, you can detect whether a property as a whole has been set using isSet on TargetConfig, but that doesn't allow you to check which individual dictionary items (such as baudrate) were set. We could make the default platform specific, but would that solve your problem?

[magnmaeh]

The way it's currently set up, you can detect whether a property as a whole has been set using isSet on TargetConfig, but that doesn't allow you to check which individual dictionary items (such as baudrate) were set. We could make the default platform specific, but would that solve your problem?

I guess it would solve my problem if the default is 9600 for Arduino and 0 for all other platforms, because 0 would in that case mean "not set". I guess the more robust solution would be having the isSet method for all dictionary items, though. If I end up implementing it for the flash property it hopefully won't be hard to extend to the other fields.

[magnmaeh]

Currently I'm struggling a bit with code size issues. When I compile a test program, e.g., TimeoutZeroThreaded, I get code sizes that already are too big to fit inside FlexPRET realized on an FPGA (since FlexPRET uses scratchpad memories (SPM), for ROM and RAM it has very little of it).

Memory region         Used Size  Region Size  %age Used
             ROM:      159108 B       256 KB     60.69%
             RAM:       31968 B       256 KB     12.19%

(This is with FlexPRET using 256 kB for instruction SPM and data SPM, which is unrealistic but I'm using it to be able to compile the programs. A more realistic choice is 64 kB for both.)

I have checked what takes up code size with nm <program> --size-sort, and see that e.g., _vfprintf_r takes up 10 kB. I have tried to track down what code pulls this function in, because it does not appear in the FlexPRET standalone applications - only the LF applications. But I have not been able to figure this out.

FlexPRET has its own standalone printf implementation, so this is just an artifact from newlib probably. So a natural solution would be to remove these newlib artifacts. That is kind of scary though, because it may have unexpected consequences down the road. But on the other hand, I don't think the FlexPRET SDK has set up newlib's printf to work properly anyway. And the test yields the same outputs before and after removing _vfprintf_r, though.

I have found two possible methods to remove these functions.

1. Add -nostdlib to compile code and manually link in object files from newlib that we actually want. This seems possible, but there might be a lot of object files to link in. And if a user in the future wants to use an additional newlib feature, they would have to link in that object file manually too. (https://stackoverflow.com/questions/38609303/how-to-add-prebuilt-object-files-to-executable-in-cmake)
2. Use the linker's --wrap option to remove certain functions. Essentially, you can write --wrap=_vfprintf_r, which will replace calls to _vfprintf_r with __wrap__vfprintf_r and rename _vfprintf_r to __real__vfprintf_r. Then we can implement __wrap__vfprintf_r as just nothing, which will not link in _vfprintf_r. (https://sourceware.org/binutils/docs-2.40/ld.html#index-_002d_002dwrap_003dsymbol)

The best option, of course, would be to figure out where in the LF source code the function gets pulled in from and see if we can filter this out for embedded systems. I found this method, but I ended up just tracking within newlib.

I tried to do 2. on the same program, and got promising results:

Memory region         Used Size  Region Size  %age Used
             ROM:      113628 B       256 KB     43.35%
             RAM:       30144 B       256 KB     11.50%

I'm not quite sure why this reduces code size by more than 10 kB (the size of the function), but probably other code becomes redundant and does not get linked in either, I guess.

I think both these options are kind of bad because they might cause headaches down the road. But they might be necessary. Do you have any thoughts @erlingrj?

[erlingrj]

I dont fully understand. Do we have our own, fully-featured printf implementation for FlexPRET? Then we should make sure that this is what is being used for the functions in util.c in reactor-c. But probably, it does not have all those features needed there. Another option would be to look in logging.h and util.c in reactor-c, and provide our own implementations of them. In fact, it would be great if that was a general possibility. I am sure other embedded targets would benefit from that also

[magnmaeh]

I think the problem is that newlib is so intertwined that when you call one of its functions, it will include references to a lot more than just that function. An example is tzset, which references _tzset_unlocked_r, which references _malloc_r and siscanf (and other functions, too). _malloc_r is already quite big (~2 kB). siscanf references __ssvfiscanf_r, which is the second biggest function I have found (6.7 kB). So from calling tzset, code size was increased by at least 8 kB. Probably much more. __ssvfiscanf_r might not ever be called, but it needs to be included in the binary because it might be. So if we "know" we never use it, we can just replace it with a stub to save a lot of code space.

I assume _vfprintf_r comes from a similar chain of references. And there are probably many newlib functions that ultimately reference _vfprintf_r too. I have not found a standardized method to find the "top-level" functions (like tzset). But if we can find such a method we can maybe try to sanitize reactor-c from these newlib functions. But that is probably a much bigger issue - I guess that would be developing a uLF like we have mentioned a few times in our meetings earlier.

But until we have a uLF, the linker wrap hack might be good enough?

[erlingrj]

The tzset probably comes from the call to ctime in the very beginning of main. I think if this was behind a macro, and likewise all the print-functions could optionally be implemented for the target platform then the code size problem would be more managable

[magnmaeh]

Ah ok, that's where tzset came from!

But for this PR it would maybe be a bit much to sanitize all of reactor-c from these function calls? Because I assume it's a big task to find all functions like ctime and replace them with a lightweight equivalent that still works. Is it ok to instead do the --wrap method I mentioned for now and create an issue on reactor-c on this very topic?

[erlingrj]

Yes, this would only affect the FlexPRET target. It would be great if you made a Github issue for this problem and linked to it in a comment where you apply the wrap workaround.

[magnmaeh]

Is this a fair description of the issue? lf-lang/reactor-c#418 (comment)

[magnmaeh]

I have a few questions: (@erlingrj / @lhstrh)

1. I replaced all fields of PlatformOptions with the Option<T> @lhstrh suggested, except for the Platform field. This is to know whether the value has been set by the user or not (as discussed here: Support for FlexPRET platform #2262 (comment)). However, many fields already had an implicit way of checking whether it had been set by the user. E.g, since board is a String it was checked against null. I have replaced everywhere I found if (board == null) with if (!board.setByUser()) and similarly for the other fields, but I am not 100% sure I have caught everything. I'm not sure how thorough CI is; maybe we can trust it to catch it if I did not fix everything?
2. I added a few build tests by just copying the RP2040 and Zephyr tests. I do not fully understand the API - so if the tests I've created do not make sense or there should be more of them, please let me know (core/src/integrationTest/java/org/lflang/tests/runtime/CFlexPRETTest.java). But is there a way to execute the tests for (emulated) embedded systems? To me it seems we can only test building for embedded platforms?
3. I'm having an issue with getting continuous standard output from an LFCommand. My command just runs a bash script, which under the hood runs a FlexPRET's emulator with the compiled LF program. The LF program normally prints out something every second, but when running it with an LFCommand, the output is only printed out once the entire program is finished. I've tried setting the verbose option on, but that does not seem to solve it. Do you have any suggestions? The relevant file (https://github.com/lf-lang/lingua-franca/pull/2262/files#diff-e74d4e3e1fc8d88fcdd4cc009c4932227bc85b0efd0e1e9bef9b3c03e87d0e16)

Once I've solved these issues I think what's left is only:

1. Verifying everything works on FPGA
2. CI for tests

[erlingrj]

I have a few questions: (@erlingrj / @lhstrh)

1. I replaced all fields of `PlatformOptions` with the `Option<T>` @lhstrh suggested, except for the `Platform` field. This is to know whether the value has been set by the user or not (as discussed here: [Add support for FlexPRET #2262 (comment)](https://github.com/lf-lang/lingua-franca/pull/2262#issuecomment-2073632803)). However, many fields already had an implicit way of checking whether it had been set by the user. E.g, since `board` is a String it was checked against `null`. I have replaced everywhere I found `if (board == null)` with `if (!board.setByUser())` and similarly for the other fields, but I am not 100% sure I have caught everything. I'm not sure how thorough CI is; maybe we can trust it to catch it if I did not fix everything?

I would say that if CI passes and you have made an effort to search it down, then we should merge it and fix any errors if they arise later on.

2. I added a few build tests by just copying the RP2040 and Zephyr tests. I do not fully understand the API - so if the tests I've created do not make sense or there should be more of them, please let me know ([core/src/integrationTest/java/org/lflang/tests/runtime/CFlexPRETTest.java](https://github.com/lf-lang/lingua-franca/pull/2262/files#diff-1f87fee5b2da3031d0ae4f826f406a7c4a7788075fc78d040a6a7a7cc1336ff6)). But is there a way to execute the tests for (emulated) embedded systems? To me it seems we can only test building for embedded platforms?

Since we have an emulator of FlexPRET we could run CI on it also. However, I don't think those tests belong here in the lingua-franca repo. I think we should just build a couple of HelloWorlds, maybe maybe run a few of them on the emulator. But it is not trivial, see next comment.

3. I'm having an issue with getting continuous standard output from an `LFCommand`. My command just runs a bash script, which under the hood runs a FlexPRET's emulator with the compiled LF program. The LF program normally prints out something every second, but when running it with an LFCommand, the output is only printed out once the entire program is finished. I've tried setting the `verbose` option on, but that does not seem to solve it. Do you have any suggestions? The relevant file (https://github.com/lf-lang/lingua-franca/pull/2262/files#diff-e74d4e3e1fc8d88fcdd4cc009c4932227bc85b0efd0e1e9bef9b3c03e87d0e16)

I don't think you can continuously monitor the output of commands, you only get it once the command terminates. I did a workaround for this in a bash script for running Zephyr tests on a QEMU emulator, since the QEMU emulator does not terminate. You will find it under .github/scripts I believe. It is a bash script that monitors the output and terminates if certain patterns are found. I don't think it is worth to do this for FlexPRET. The reason is that it is a little brittle, and FlexPRET has a big stack of dependencies and if some test for some reason fails, it is more likely to be a problem in the CPU design or the SDK than in LFC or reactor-c.

Once I've solved these issues I think what's left is only:

1. Verifying everything works on FPGA

2. CI for tests

Great!

[lhstrh]

This looks good now, with the only caveat that there are some tests that are "expected to pass" as part of the merge requirements but have now been moved. We can bypass the branch protection once we're ready to merge and then adjust the GitHub configuration.

[magnmaeh]

Thank you for fixing my typo! :)

lf-lang/lf-lang.github.io#254 is now ready for review as well :) I think the only things left are:

• Review of Add flexpret docs lf-lang.github.io#254
• Transferring ownership of https://github.com/magnmaeh/lf-flexpret-template to lf-lang
• Fixing link in docs to point to the lf-lang lf-flexpret-template (I assume it's going to be https://github.com/lf-lang/lf-flexpret-template, though)
• Merge Remove lingua-franca submodule lf-flexpret-template#1
• Comment in this part: https://github.com/lf-lang/reactor-c/pull/412/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03f

[erlingrj]

I think this looks very good. Just a few minor suggestions

[magnmaeh]

@lhstrh I tried to transfer ownership of https://github.com/magnmaeh/lf-flexpret-template but I need access to create public repos on lf-lang to do that.

[lhstrh]

@magnmaeh: sent you an invite to join the org!

[lhstrh]

LGMT, modulo some nitpicks.

[erlingrj]

The failure in TS is tracked here #2293, I think we should merge even with this failure and this must be solved independently.

The last issue is that zephyr and arduino tests appear as required but are not run:

I don't really see why, those workflows are not references anywhere except in the c-embedded.yml (which runs) and in the reactor-c CI

[lhstrh]

The failure in TS is tracked here #2293, I think we should merge even with this failure and this must be solved independently.

The last issue is that zephyr and arduino tests appear as required but are not run:

I don't really see why, those workflows are not references anywhere except in the c-embedded.yml (which runs) and in the reactor-c CI

It's configured in the GitHub settings. This needs to be changed because the workflows have moved around. I'll take care of this.