-
Notifications
You must be signed in to change notification settings - Fork 211
Add convenience proc-macros #553
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.
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
base: master
Are you sure you want to change the base?
Conversation
This is also my understanding. But whether "the CPU will fault" = "ok" is a big question? Again, the example with the SPI driver. It is absolutely not "ok" to rely on coincidence and have "some items in flash" and "some in sram". Because then, the async SPI driver will end up crashing, and people with start complaining that "it does not work". And then go explain the whole saga that putting stuff in IRAM is best effort only. And they might even not know about "IRAM" in the first place. Which is why the SPI driver's async code-path is compiled out when the ESP-IDF config CONFIG_SPI_ISR_HANDLER_IN_IRAM_BLAH_BLAH is enabled. Which folks find non-intuitive but that's another topic: #486 For other cases - like the just-merged PCNT driver - not having those counting functions in IRAM only means that they will be executed more slowly, and might report big count gaps if somebody is locking the flash and operating on it at the same time (i.e. an OTA update or suchlike). But this is anyway also valid for the user code which calls the driver count functions - if it is not in IRAM (and this is very difficult to assert) it will also be delayed when counting, even if the driver API itself is not. === So yeah, we can have the macros you suggest, but their usefulness might be a bit limited, as per above? |
|
It would be ideal if the macro could be used to put the function and everything it requires into SRAM, but I don't know how that could be accomplished without placing the I thought about going the linker route mentioned in the docs, which would circumvent some of the problems, but I am not knowledgeable enough about the way esp projects are built in rust. Regarding the fallout of partially having items in flash and sram:This is the issue the docs describe. By default the interrupts seem to be not IRAM-safe and they will be suspended while the flash is accessed. To let the interrupt execute while it is accessing the flash, one must set the |
That's the point exactly. You have to place it on the transitive closure of what the function calls, and (a) it might not work for some functions (generics, who knows what, etc.) or (b) it might even be something you don't have access to (3rd party crates etc.).
Sigh. I know that. But - not all drivers give you the freedom to say - with an API call - "hey, driver, your interrupt handlers must be iram-safe". For some, this is a conf setting. Hence the SPI async driver CONF_ drama from above. Look at the issue I've linked. ==== With that said, if you want to push this to completion - sure. Just don't be fooled that this is a universal, generic solution to the "your code shall be in IRAM, completely" problem. |
Wasn't my intention. The rust compiler is not even placing string literals in the right section, only with some const code hacks. The magic solution for this, is likely far away. It is supposed to be an initial attempt at being able to place things in iram, similarly to what the IRAM_ATTR in the C code does, cleaning up the uses in the code where currently link_section is manually specified. If someone has the ambition to improve upon this in the future, sure, but I hope I am not the one who needs it. |
|
After thinking about this, and doing some research, I want to spend some more time exploring the linker router. (I will make a draft PR out of this) Main idea would be to do it like this: but I will have to think a bit about how it could work out |
|
Okay, I spent the time looking at the problem. The ESP-IDF linker fragments allow putting a symbol, object or entire (static) library into ram. This behavior can be replicated in rust through Assuming this would be implemented, one would have to separate the codebase into one for ram code and one for all the other code. I think this is quite inconvenient, and a proc-macro wouldn't help here. This seems like it should be a template, with maybe a crate that contains all the build.rs code. But implementing this is not as easy as it sounds, given that the standard library exists and will likely be invoked. If it is compiled as a static library, the standard library will be fully exported, preventing it from being compiled with the flash code (that would also have the standard library in it -> symbol conflicts). Compiling as a dynamic library is another option, but here I am wondering how (assuming one knows which functions should be in ram) one would put the std functions in ram. Transitively putting code in some sections sounds like a tricky thing to accomplish and I think the people who really need that should write these interrupt handlers in c code or spend the time looking at the assembly and ensuring that it is not accessing flash. |
|
Right. This is why I gave up on this long ago. |
Dynamic libraries are not supported on MCUs. |
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo fmtcommand to ensure that all changed code is formatted correctly.cargo clippycommand to ensure that all changed code passes latest Clippy nightly lints.CHANGELOG.mdin the proper section.Pull Request Details 📖
Description
After spending the time writing the issue #552, I wanted to see if it works (yes it does), given that it should be trivial to implement, but it wasn't trivial (like every time I try to implement something for the esp).
Here are my findings:
IRAM_ATTRwill link code to.iram1, which will end up in.iram0.textof the final binary.dram1,.dram0.datain the final binary) which is used for data.__COUNTER__to put each element into a unique subsection, in the final binary they will all end up in.iram0.text/.dram0.data. These unique subsections seem to be for some linker issue that I could not reproduce in rust. I added code that tries to put each one in a distinct subsection. If there are multiple items in the same section, it shouldn't be in an issue in rust, because all items are mangled by default.#[link_section = ...]will not place literals in the section. See Place &str in specific linker sections rust-lang/rust#70239, I added some workaround code for this issue that took up the majority of time.I also added an internal helper macro that should help with documenting the features required for a piece of code #547.
What could be added in the future?
Testing
Simply use the crate in the project, then you can list all symbols in the final binary with
xtensa-esp32-elf-objdump.exe -x your_binary(if mangling is on, search with the regex\.iram0\.text.*? _ZN). To look at what was stored there, I usedxtensa-esp32-elf-readelf -x .dram0.dataand looked up the address there.