Configuration option to disable static inline on functions #986

Open
opened 2022-08-23 18:38:11 -05:00 by KizzyCode · 9 comments
KizzyCode commented 2022-08-23 18:38:11 -05:00 (Migrated from github.com)

Would it be possible to replace the static inlines with a configurable attribute that can be disabled if necessary?

Background

We are using a Rust library to implement the main logic. The lib defines a pub extern "C" fn rust_entry() which gets called from the C main function and then takes over control and calls back into the Pico-SDK where necessary (e.g. for malloc/free, GPIO etc.)

This concept works amazingly well – except for one problem: Since a lot of the SDK's functions are static, we cannot link against them. Our current workaround is to implement a 1:1 C-wrapper for all the static functions we need – however this gets quite cumbersome over time.

A possible naive solution could be sth. like

#ifndef PICO_DISABLE_STATIC_INLINE
    #define __pico_static_inline static inline
#else
    #define __pico_static_inline
#endif

and then using __pico_static_inline on all functions (if it is not crucial that they are inlined of course).

I know that our setup is probably a little bit exotic and I'm completely fine if this issue gets closed 😅
However IMO this could ease the burden to write an application in language-of-your-choice, because if you can link back against the SDK, you suddenly don't need an entire native toolchain/SDK in language-of-your-choice anymore to get stuff running.

Would it be possible to replace the `static inline`s with a configurable attribute that can be disabled if necessary? ## Background We are using a Rust library to implement the main logic. The lib defines a `pub extern "C" fn rust_entry()` which gets called from the C `main` function and then takes over control and calls back into the Pico-SDK where necessary (e.g. for `malloc`/`free`, GPIO etc.) This concept works amazingly well – except for one problem: Since a lot of the SDK's functions are `static`, we cannot link against them. Our current workaround is to implement a 1:1 C-wrapper for all the `static` functions we need – however this gets quite cumbersome over time. A possible naive solution could be sth. like ```c #ifndef PICO_DISABLE_STATIC_INLINE #define __pico_static_inline static inline #else #define __pico_static_inline #endif ``` and then using `__pico_static_inline` on all functions (if it is not crucial that they are inlined of course). I know that our setup is probably a little bit exotic and I'm completely fine if this issue gets closed 😅 However IMO this could ease the burden to write an application in _language-of-your-choice_, because if you can link back against the SDK, you suddenly don't need an entire native toolchain/SDK in _language-of-your-choice_ anymore to get stuff running.
lurch commented 2022-08-23 19:28:54 -05:00 (Migrated from github.com)

I've never written any Rust myself, and I don't know enough about the C SDK to answer your question, but have you had a look at https://github.com/rp-rs ? ping @thejpster

I've never written any Rust myself, and I don't know enough about the C SDK to answer your question, but have you had a look at https://github.com/rp-rs ? ping @thejpster
KizzyCode commented 2022-08-23 19:37:59 -05:00 (Migrated from github.com)

Yes, I know about rp-rs; nevertheless tyvm for the suggestion :D

There are a few reasons why we decided to stick with the official SDK if possible (e.g. pretty stable, quick bugfixes for breaking issues, has a lot of built-in features etc. – we found that even if another toolchain suits the needs we have now there is a significant risk that we well run into problems/missing features in the future).

Yes, I know about `rp-rs`; nevertheless tyvm for the suggestion :D There are a few reasons why we decided to stick with the official SDK if possible (e.g. pretty stable, quick bugfixes for breaking issues, has a lot of built-in features etc. – we found that even if another toolchain suits the needs we have _now_ there is a significant risk that we well run into problems/missing features in the future).
Memotech-Bill commented 2022-08-24 08:09:18 -05:00 (Migrated from github.com)

I ran into a similar problem providing access to the SDK routines from BBC BASIC. My solution was to write some Python code which reads through the SDK header files and automatically generates wrapper code such as:

#include "pico/types.h"

uint64_t stub_to_us_since_boot (absolute_time_t t)
    {
    return to_us_since_boot (t);
    }

void stub_update_us_since_boot (absolute_time_t *t, uint64_t us_since_boot)
    {
    update_us_since_boot (t, us_since_boot);
    }

The stub_ functions are then callable. The BBC BASIC function look-up (SYS) hides the stub_ prefix from the user.

Since the wrapper functions are automatically generated they can be reproduced from the latest SDK at every build.

My problem is with LWIP which makes extensive use of C #define macros. These cannot be automatically stubbed as the parameter and return types are unknown.

I ran into a similar problem providing access to the SDK routines from BBC BASIC. My solution was to write some [Python code](https://github.com/Memotech-Bill/PicoBB/blob/lwip_raw_tcp/src/pico/symbols/header_funcs.py) which reads through the SDK header files and automatically generates wrapper code such as: ``` #include "pico/types.h" uint64_t stub_to_us_since_boot (absolute_time_t t) { return to_us_since_boot (t); } void stub_update_us_since_boot (absolute_time_t *t, uint64_t us_since_boot) { update_us_since_boot (t, us_since_boot); } ``` The stub_ functions are then callable. The BBC BASIC function look-up (SYS) hides the stub_ prefix from the user. Since the wrapper functions are automatically generated they can be reproduced from the latest SDK at every build. My problem is with LWIP which makes extensive use of C #define macros. These cannot be automatically stubbed as the parameter and return types are unknown.
KizzyCode commented 2022-08-24 08:38:39 -05:00 (Migrated from github.com)

@Memotech-Bill Uhh, this looks pretty nice – I'll definitively take a closer look at it, this could be a good workaround!

@Memotech-Bill Uhh, this looks pretty nice – I'll definitively take a closer look at it, this could be a good workaround!
kilograham commented 2022-08-24 09:32:30 -05:00 (Migrated from github.com)

This is potentially useful for people who want to make a static lib of the SDK (although #882 also helps)

This is potentially useful for people who want to make a static lib of the SDK (although #882 also helps)
michael-ring commented 2022-08-24 09:40:37 -05:00 (Migrated from github.com)

@Memotech-Bill , really nice idea, I am fighting the same issue with my freepascal port of the pico headers, I ended up re-implementing lots of the static inlines with pascal code.

@Memotech-Bill , really nice idea, I am fighting the same issue with my freepascal port of the pico headers, I ended up re-implementing lots of the static inlines with pascal code.
thejpster commented 2022-08-24 11:41:54 -05:00 (Migrated from github.com)

My 2p: Rather than disabling static wholesale, it seems to make sense to go through the API and decide what is public (and hence something you are careful not to break without warning people) and what is internal only. The internal APIs should not have extern linkage (i.e. remain static), and the public API should have extern linkage (i.e. not be marked static).

If application developers rely on APIs that are internal only, you should consider stabilizing them and making them public.

If the issue is that the static inline functions are trivial things defined (and not just declared) in the header file (ugh), then this kind of macro is probably the right work-around (also, don't do that and consider enabling LTO instead).

My 2p: Rather than disabling static wholesale, it seems to make sense to go through the API and decide what is public (and hence something you are careful not to break without warning people) and what is internal only. The internal APIs should not have extern linkage (i.e. remain static), and the public API should have extern linkage (i.e. not be marked static). If application developers rely on APIs that are internal only, you should consider stabilizing them and making them public. If the issue is that the `static inline` functions are trivial things defined (and not just declared) in the header file (ugh), then this kind of macro is probably the right work-around (also, don't do that and consider enabling LTO instead).
kilograham commented 2022-08-24 12:47:34 -05:00 (Migrated from github.com)

Yeah this is just referring to public API functions which happen to implemented inline for code size/speed reasons.

And now i give it more than a moments thought, having something to generate is better, because either
a) you remove the static as well as the inline, in which case you'll get multiple-definitions
b) you leave the static, and you get duplicate code, but also no public callable version

Yeah this is just referring to public API functions which happen to implemented inline for code size/speed reasons. And now i give it more than a moments thought, having something to generate is better, because either a) you remove the static as well as the inline, in which case you'll get multiple-definitions b) you leave the static, and you get duplicate code, but also no public callable version
KizzyCode commented 2022-09-12 19:47:53 -05:00 (Migrated from github.com)

I'm a little bit torn between both options...
On one side, generating something is much easier and faster; on the other side – @thejpster has a point: Using static inline within headers for public APIs is non-idiomatic C and breaks FFI compatibility.

Furthermore I'm wondering if static inline is necessary? Doing LTO is probably a much better approach and has the additional advantage that it can be tweaked for either performance or code size, whereas static inline always optimizes for performance.

For those interested: As a stopgap I've created a tiny helper to generate no-static no-inline bindings for the static inline functions. It's a bit cursed because it uses regex to parse the headers, but it works surprisingly well 😅
There are also pregenerated bindings in generated/.

I'm a little bit torn between both options... On one side, generating something is much easier and faster; on the other side – @thejpster has a point: Using `static inline` within headers for public APIs is non-idiomatic C and breaks FFI compatibility. Furthermore I'm wondering if `static inline` is necessary? Doing LTO is probably a much better approach and has the additional advantage that it can be tweaked for either performance or code size, whereas `static inline` always optimizes for performance. For those interested: As a stopgap I've created a [tiny helper](https://github.com/KizzyCode/PicoSDK-NoStatic-Bindgen-Python) to generate no-`static` no-`inline` bindings for the `static inline` functions. It's a bit cursed because it uses regex to parse the headers, but it works surprisingly well 😅 There are also pregenerated bindings in `generated/`.
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk#986
No description provided.