[PATCH] x86: help inlining of functions involving alternative_call()

Jan Beulich posted 1 patch 2 years, 3 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/09369d40-a974-2f7a-9b70-836b9210904b@suse.com
There is a newer version of this series
[PATCH] x86: help inlining of functions involving alternative_call()
Posted by Jan Beulich 2 years, 3 months ago
The involved asm() expands to large enough a construct that often the
compiler would decide against inlining when a containing function is
used more than once in a CU. Use the "inline" keyword when supported by
the compiler in conjunction with asm().

The INIT_SECTIONS_ONLY dependency is because in that case "inline" gets
expanded to "__inline__ __init", which obviously can't be used with
asm(). But for init-time only code we're also less worried ...

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Perhaps this wants extending to other asm()-s involving ALTERNATIVE().
At which point the question is whether instead of altcall_asm we'd want
to have something line asm_inline.

I understand that Linux uses Kconfig for the feature detection. The
discussion on whether we want to generally follow this model continues
to be pending / stalled. In this particular case the error output from
the compiler when "inline" is not supported can be huge (thousands of
lines, partly because of the many nested levels of macro expansions),
making it close to impossible to recognize what the actual issue is. Yet
that's what would be happening if one switched the compiler from one
supporting the feature to one not supporting it, without remembering to
explicitly have xen/.config re-generated.

--- a/xen/Makefile
+++ b/xen/Makefile
@@ -250,6 +250,7 @@ CFLAGS += -Werror -Wredundant-decls -Wno
 $(call cc-option-add,CFLAGS,CC,-Wvla)
 CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
 CFLAGS-$(CONFIG_DEBUG_INFO) += -g
+CFLAGS-$(call success, echo 'void _(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null) += -DCC_HAS_ASM_INLINE
 
 ifneq ($(CONFIG_CC_IS_CLANG),y)
 # Clang doesn't understand this command line argument, and doesn't appear to
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -203,11 +203,17 @@ extern void alternative_branches(void);
 #define ALT_CALL6_OUT "+r" (a1_), "+r" (a2_), "+r" (a3_), \
                       "+r" (a4_), "+r" (a5_), "+r" (a6_)
 
+#if defined(INIT_SECTIONS_ONLY) || !defined(CC_HAS_ASM_INLINE)
+# define altcall_asm asm volatile
+#else
+# define altcall_asm asm volatile inline
+#endif
+
 #define alternative_callN(n, rettype, func) ({                     \
     rettype ret_;                                                  \
     register unsigned long r10_ asm("r10");                        \
     register unsigned long r11_ asm("r11");                        \
-    asm volatile (ALTERNATIVE("call *%c[addr](%%rip)", "call .",   \
+    altcall_asm ( ALTERNATIVE("call *%c[addr](%%rip)", "call .",   \
                               X86_FEATURE_ALWAYS)                  \
                   : ALT_CALL ## n ## _OUT, "=a" (ret_),            \
                     "=r" (r10_), "=r" (r11_) ASM_CALL_CONSTRAINT   \