[v1] cpu_x86: Do not inline cpuidCall()

[PATCH] cpu_x86: Do not inline cpuidCall()

Posted by Fabio Estevam 8 months, 4 weeks ago

The following build error is observed with DEBUG_BUILD = 1:

src/cpu/cpu_x86.c: In function 'cpuidSetLeaf4':
src/cpu/cpu_x86.c:2563:1: error: inlining failed in call to 'cpuidCall': function not considered for inlining [-Werror=inline]
 2563 | cpuidCall(virCPUx86CPUID *cpuid)
      | ^~~~~~~~~

Remove the 'inline' specifier to avoid the problem.

Reported-by: Hongxu Jia <hongxu.jia@windriver.com>
Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 src/cpu/cpu_x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 213af67ea478..0f7eb8f48b35 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2564,7 +2564,7 @@ virCPUx86DataCheckFeature(const virCPUData *data,
 
 
 #if defined(__i386__) || defined(__x86_64__)
-static inline void
+static void
 cpuidCall(virCPUx86CPUID *cpuid)
 {
     virHostCPUX86GetCPUID(cpuid->eax_in,
-- 
2.34.1

Re: [PATCH] cpu_x86: Do not inline cpuidCall()

Posted by Ján Tomko via Devel 8 months, 3 weeks ago

On a Wednesday in 2025, Fabio Estevam wrote:
>The following build error is observed with DEBUG_BUILD = 1:
>

What exactly is DEBUG_BUILD? I can't find the variable anywhere.

--buildtype debug is the default for meson
with -Doptimization=g this should already be disabled since:
commit 7253dda5178e0504a4f3ba859c96883aca6bec7d
     build-sys: drop -Winline when optimization=g

>src/cpu/cpu_x86.c: In function 'cpuidSetLeaf4':
>src/cpu/cpu_x86.c:2563:1: error: inlining failed in call to 'cpuidCall': function not considered for inlining [-Werror=inline]
> 2563 | cpuidCall(virCPUx86CPUID *cpuid)
>      | ^~~~~~~~~
>
>Remove the 'inline' specifier to avoid the problem.
>

Is there a point in having
1) -Winline
2) inline functions at all (outside of header files)?

>Reported-by: Hongxu Jia <hongxu.jia@windriver.com>
>Signed-off-by: Fabio Estevam <festevam@gmail.com>
>---
> src/cpu/cpu_x86.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
>index 213af67ea478..0f7eb8f48b35 100644
>--- a/src/cpu/cpu_x86.c
>+++ b/src/cpu/cpu_x86.c
>@@ -2564,7 +2564,7 @@ virCPUx86DataCheckFeature(const virCPUData *data,
>
>
> #if defined(__i386__) || defined(__x86_64__)
>-static inline void
>+static void
> cpuidCall(virCPUx86CPUID *cpuid)
> {
>     virHostCPUX86GetCPUID(cpuid->eax_in,

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

Re: [PATCH] cpu_x86: Do not inline cpuidCall()

Posted by Fabio Estevam 8 months, 3 weeks ago

Hi Ján,

On Fri, May 16, 2025 at 8:30 AM Ján Tomko <jtomko@redhat.com> wrote:
>
> On a Wednesday in 2025, Fabio Estevam wrote:
> >The following build error is observed with DEBUG_BUILD = 1:
> >
>
> What exactly is DEBUG_BUILD? I can't find the variable anywhere.

DEBUG_BUILD is a configuration variable used in OpenEmbedded:

https://docs.yoctoproject.org/ref-manual/variables.html#term-DEBUG_BUILD

I have just sent a v2 explaining this in the commit log.

Re: [PATCH] cpu_x86: Do not inline cpuidCall()

Posted by Daniel P. Berrangé via Devel 8 months, 3 weeks ago

On Fri, May 16, 2025 at 01:30:41PM +0200, Ján Tomko via Devel wrote:
> On a Wednesday in 2025, Fabio Estevam wrote:
> > The following build error is observed with DEBUG_BUILD = 1:
> > 
> 
> What exactly is DEBUG_BUILD? I can't find the variable anywhere.
> 
> --buildtype debug is the default for meson
> with -Doptimization=g this should already be disabled since:
> commit 7253dda5178e0504a4f3ba859c96883aca6bec7d
>     build-sys: drop -Winline when optimization=g
> 
> > src/cpu/cpu_x86.c: In function 'cpuidSetLeaf4':
> > src/cpu/cpu_x86.c:2563:1: error: inlining failed in call to 'cpuidCall': function not considered for inlining [-Werror=inline]
> > 2563 | cpuidCall(virCPUx86CPUID *cpuid)
> >      | ^~~~~~~~~
> > 
> > Remove the 'inline' specifier to avoid the problem.
> > 
> 
> Is there a point in having
> 1) -Winline

Well this caught this bogus use of "inline" in a source
file, but arguably we could do that in a syntax-check
too.

> 2) inline functions at all (outside of header files)?

No, IMHO, the only valid place to use 'inline' is for
the 'static inline'  declaration of a function in a
header file. Anywhere else just let the compiler
optimization decide what to do - it knows the situation
far better that the author will.

> 
> > Reported-by: Hongxu Jia <hongxu.jia@windriver.com>
> > Signed-off-by: Fabio Estevam <festevam@gmail.com>
> > ---
> > src/cpu/cpu_x86.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> > index 213af67ea478..0f7eb8f48b35 100644
> > --- a/src/cpu/cpu_x86.c
> > +++ b/src/cpu/cpu_x86.c
> > @@ -2564,7 +2564,7 @@ virCPUx86DataCheckFeature(const virCPUData *data,
> > 
> > 
> > #if defined(__i386__) || defined(__x86_64__)
> > -static inline void
> > +static void
> > cpuidCall(virCPUx86CPUID *cpuid)
> > {
> >     virHostCPUX86GetCPUID(cpuid->eax_in,
> 
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
> 
> Jano



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|