RE: [RFC] doc, devtools: discourage new __rte_always_inline

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, 27 May 2026 18.38
> 
> Modern compilers at -O2 make good inlining decisions for small
> static inline functions; forced inlining via __rte_always_inline
> should be reserved for cases where it is required for correctness
> or for documented measured performance reasons.
> 
> Document the policy in the coding style guide and add a
> checkpatches.sh entry that warns when new uses of the attribute
> are introduced.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Plenty of code in DPDK is based on ancient compiler behavior; let's not add new code based on old compiler behavior.
I guess developers keep blindly adding __rte_always_inline, only because it's already everywhere.

Good initiative to stop that bad habit.
Slightly extreme forbidding it in checkpatches.sh, but it's probably the only way to force people to stop and think.

> ---
>  devtools/checkpatches.sh                 |  8 ++++++++
>  doc/guides/contributing/coding_style.rst | 25 +++++++++++++++++++++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index f5dd77443f..2a3d364178 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -137,6 +137,14 @@ check_forbidden_additions() { # <patch>
>  		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk
> \
>  		"$1" || res=1
> 
> +	# forbid new use of __rte_always_inline
> +	awk -v FOLDERS="lib drivers app examples" \
> +		-v EXPRESSIONS='\\<__rte_always_inline\\>' \
> +		-v RET_ON_FAIL=1 \
> +		-v MESSAGE='Adding __rte_always_inline; prefer plain
> inline' \
> +		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk
> \
> +		"$1" || res=1
> +
>  	# refrain from using compiler __rte_atomic_thread_fence()
>  	# It should be avoided on x86 for SMP case.
>  	awk -v FOLDERS="lib drivers app examples" \
> diff --git a/doc/guides/contributing/coding_style.rst
> b/doc/guides/contributing/coding_style.rst
> index 243a3c2959..97e459853c 100644
> --- a/doc/guides/contributing/coding_style.rst
> +++ b/doc/guides/contributing/coding_style.rst
> @@ -747,11 +747,34 @@ Static Variables and Functions
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
>  * All functions and variables that are local to a file must be
> declared as ``static`` because it can often help the compiler to do
> some optimizations (such as, inlining the code).
> -* Functions that should be inlined should to be declared as ``static
> inline`` and can be defined in a .c or a .h file.
> +* Functions that should be inlined should be declared as ``static
> inline`` and can be defined in a .c or a .h file.
> 
>  .. note::
>  	Static functions defined in a header file must be declared as
> ``static inline`` in order to prevent compiler warnings about the
> function being unused.
> 
> +Use of ``__rte_always_inline``
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The ``__rte_always_inline`` attribute forces the compiler to inline a
> function regardless of its size or call-graph heuristics.
> +Prefer plain ``inline`` (or no annotation at all for static functions)
> and let the compiler decide.
> +Modern compilers at ``-O2`` make good inlining decisions for small
> ``static inline`` functions in headers,
> +and forced inlining can hurt performance by inflating function bodies,
> increasing register pressure, and overriding profile-guided
> optimization.
> +
> +``__rte_always_inline`` should only be used when one of the following
> applies:
> +
> +* The function contains ``__builtin_constant_p`` checks that gate a

__builtin_constant_p -> __rte_constant

> constant-folded fast path, and the optimization is lost if the function
> is not inlined into the caller.
> +  Examples include byte-order helpers and length-dispatched
> copy/compare routines.
> +
> +* The function wraps inline assembly or a compiler intrinsic whose
> correctness depends on being inlined into the caller's register context
> (for example, intrinsics requiring a compile-time constant argument).
> +
> +* Measurement on a representative workload shows that the annotation
> is required to retain performance, and the reason is documented in the
> commit message that introduces it.
> +
> +Each use must be justified at the point it is introduced. Adding
> ``__rte_always_inline`` because nearby code uses it is not a
> justification;
> +if the constant or intrinsic that requires inlining is several call
> levels up the call chain,
> +restructure the code rather than annotating the entire chain.

Please add something similar to the description of the macro in rte_common.h.

> +
> +The complementary attribute ``__rte_noinline`` is useful for
> explicitly marking cold paths (error handling, initialization, slow-
> path fallbacks) where outlining the function can reduce instruction-
> cache pressure on the hot path.

Same comment:
Please add something similar to the description of the macro in rte_common.h.

<feature creep>
Many macros in rte_common.h have insufficient descriptions.
</feature creep>

> +
>  Const Attribute
>  ~~~~~~~~~~~~~~~
> 
> --
> 2.53.0

With comments above fixed,
Acked-by: Morten Brørup <mb@smartsharesystems.com>