Consider FD_INLINE macro about firedancer HOT 7 CLOSED

2 >~ 3 >> 1

That is:

2 is the clunky traditional solution to the horrors of inline here but it will "just work".

3 is more modern. Due to the past horrors though, my knee-jerk and possibly dated reaction is to just assume extern inline isn't a "thing" in the real world. But, if tools have caught up to it such that it "just works" too nowadays, open to it.

1 feels like an option of last resort. PITA for software maintainability with a lots of nasty bug surface area.

from firedancer.

IMO this is a good solution. It removes the maintenance burden of re-implementing the C functions in Rust and gives us the flexibility to override this behaviour on a case-by-case basis for instances where performance becomes a problem.

from firedancer.

inline has historically been a giant mess in the C/C++ language. Standard support was between non-existent to spotty. What was there had parts that were technically burdensome enough at the time (extern inline) to make compiler conformance incomplete at best (and some of the FFI stuff has a flavor of this).

But there was lots of legit developer demand for it and, to meet this demand, language and compiler extensions (often contradictory) sprung up. Compiler's honoring whatever support they did have was dicey at best though ... excerpting a rant from the header documentation:

/* FD_FN_UNUSED indicates that it is okay if the function with static
   linkage is not used.  Allows working around -Winline in header only
   APIs where the compiler decides not to actually inline the function.
   (This belief, frequently promulgated by anti-macro cults, that "An
   Inline Function is As Fast As a Macro" ... an entire section in gcc's
   documentation devoted to it in fact ... remains among the biggest
   lies in computer science.  Yes, an inline function is as fast as a
   macro ... when the compiler actually decides to treat the inline
   keyword more than just for entertainment purposes only.  Which, as
   -Winline proves, it frequently doesn't.  Sigh ... force_inline like
   compiler extensions might be an alternative here but they have their
   own portability issues.) */

#define FD_FN_UNUSED __attribute__((unused))

This was encouraged by C++ code doing lots of implementation in header files and overusing inline (lots of developer blindness to the L1I instruction cache, instruction fetch and TLB implications ... which the compiler writers partially compensated in the optimizer by more or less overriding the developers intent when they felt like it ... for the record, I have a rather strong dislike of macros but dislike the compiler disregarding developer's intent even more). So, eventually C/C++ got around to making inline better linguistically supported but still a crap shoot under the hood.

With all that said, the historical practice in various spiritual predecessors to FD's development environment was to macro it up exactly as suggested. I've been trying to remove as many workarounds to historical linguistic gotchas as possible in FD since many of the idioms are compensating for issues going back decades. It'd be nice if we could just say static inline these days and get the desired behavior. But if the languages are still decades behind where they need to be, so be it.

Also with that, #define FD_INLINE static inline is definitely wrong as there are some very real differences between what inline means and static inline means. So at the least this macro should either be a function modifier like #define FD_FN_INLINE or maybe (closer to historical practice), #define FD_STATIC_INLINE to make it explicit that the behavior expected is like ... uh ... static inline.

I'm also open to games with attributes like force_inline and/or FD_FN_UNUSED for a macro solution here. The current situation helps clue in developers that inline is not gospel to the compiler when they compile with -Winline. But, if we are about to conclude that inline is still a giant pile of fail linguistically, we could go the distance and do something far more aggressive to get the language and compiler to honor behave deterministically (might have a cost if we ever wanted to run outside gcc and/or clang-ish tool chains though).

from firedancer.

The workaround is basically equivalent to #2, the build tooling parses the C headers, locates static inlines, and adds an extern wrapper for them. A little extra magic wires everything for free.

from firedancer.

cc @kbowers-jump @tpointon-jump @leoluk curious if you have any thoughts

from firedancer.

Also with that, #define FD_INLINE static inline is definitely wrong as there are some very real differences between what inline means and static inline means. So at the least this macro should either be a function modifier like #define FD_FN_INLINE or maybe (closer to historical practice), #define FD_STATIC_INLINE to make it explicit that the behavior expected is like ... uh ... static inline.

@kbowers-jump Great point, I like the idea of FD_STATIC_INLINE.

The issue I'm trying to solve with this change is to create a translation unit that includes "fallback" function definitions of all exported "inline functions" but still inline those at all the C call sites (current behavior). This allows FFI code to use that fallback as real inlining is not implemented.

This sounds exactly what extern inline combined with __attribute__((force_inline)) might be able to provide.

I appreciate the context but it sounds like both the FD_STATIC_INLINE macro switch and the extern inline choices are broken in C compilers. (For the record, in Rust, this is even worse)

So of the three suggested choices, what is your suggestion to proceed? (For the Tango fns specifically, like querying fields from meta)

1. Reimplement inline in Rust
2. FD_STATIC_INLINE toggle
3. extern inline and hack the build system to work around it

from firedancer.

Closing this as resolved. We didn't add an FD_INLINE macro, but @mmcgee-jump found a workaround to provide static inlines to FFI.

from firedancer.