[PATCH v2] x86/boot: Optimise 32 bit C source code

Frediano Ziglio posted 1 patch 2 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20240909104402.67141-1-frediano.ziglio@cloud.com
xen/arch/x86/boot/Makefile | 1 +
1 file changed, 1 insertion(+)
[PATCH v2] x86/boot: Optimise 32 bit C source code
Posted by Frediano Ziglio 2 months, 2 weeks ago
The various filters are removing all optimisations.
No need to have all optimisations turned off.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 xen/arch/x86/boot/Makefile | 1 +
 1 file changed, 1 insertion(+)
---
Changes since v1
- reuse optimization level from XEN_CFLAGS.

diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index 8f5bbff0cc..8352ce023b 100644
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -16,6 +16,7 @@ $(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
 CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
 CFLAGS_x86_32 += -nostdinc -include $(filter %/include/xen/config.h,$(XEN_CFLAGS))
 CFLAGS_x86_32 += $(filter -I%,$(XEN_CFLAGS))
+CFLAGS_x86_32 += $(filter -O%,$(XEN_CFLAGS))
 
 # override for 32bit binaries
 $(head-bin-objs): CFLAGS_stack_boundary :=
-- 
2.34.1
Re: [PATCH v2] x86/boot: Optimise 32 bit C source code
Posted by Andrew Cooper 2 months, 2 weeks ago
On 09/09/2024 11:44 am, Frediano Ziglio wrote:
> The various filters are removing all optimisations.
> No need to have all optimisations turned off.
>
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> ---
>  xen/arch/x86/boot/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> ---
> Changes since v1
> - reuse optimization level from XEN_CFLAGS.
>
> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> index 8f5bbff0cc..8352ce023b 100644
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -16,6 +16,7 @@ $(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
>  CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
>  CFLAGS_x86_32 += -nostdinc -include $(filter %/include/xen/config.h,$(XEN_CFLAGS))
>  CFLAGS_x86_32 += $(filter -I%,$(XEN_CFLAGS))
> +CFLAGS_x86_32 += $(filter -O%,$(XEN_CFLAGS))

I'm pretty sure this can be part of the prior expression,

CFLAGS_x86_32 += $(filter -I% -O%,$(XEN_CFLAGS))

Happy to fix on commit.

~Andrew
Re: [PATCH v2] x86/boot: Optimise 32 bit C source code
Posted by Frediano Ziglio 2 months, 2 weeks ago
On Mon, Sep 9, 2024 at 11:47 AM Andrew Cooper <andrew.cooper3@citrix.com>
wrote:

> On 09/09/2024 11:44 am, Frediano Ziglio wrote:
> > The various filters are removing all optimisations.
> > No need to have all optimisations turned off.
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> > ---
> >  xen/arch/x86/boot/Makefile | 1 +
> >  1 file changed, 1 insertion(+)
> > ---
> > Changes since v1
> > - reuse optimization level from XEN_CFLAGS.
> >
> > diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> > index 8f5bbff0cc..8352ce023b 100644
> > --- a/xen/arch/x86/boot/Makefile
> > +++ b/xen/arch/x86/boot/Makefile
> > @@ -16,6 +16,7 @@ $(call
> cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
> >  CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
> >  CFLAGS_x86_32 += -nostdinc -include $(filter
> %/include/xen/config.h,$(XEN_CFLAGS))
> >  CFLAGS_x86_32 += $(filter -I%,$(XEN_CFLAGS))
> > +CFLAGS_x86_32 += $(filter -O%,$(XEN_CFLAGS))
>
> I'm pretty sure this can be part of the prior expression,
>
> CFLAGS_x86_32 += $(filter -I% -O%,$(XEN_CFLAGS))
>
> Happy to fix on commit.
>
>
>
No objections.

On a similar subject we don't pass -D__XEN__ to these files, this looks
wrong to me, they are Xen sources.
Should I send a separate patch after this with that change?

Frediano
Re: [PATCH v2] x86/boot: Optimise 32 bit C source code
Posted by Andrew Cooper 2 months, 2 weeks ago
On 09/09/2024 11:51 am, Frediano Ziglio wrote:
> On Mon, Sep 9, 2024 at 11:47 AM Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>
>     On 09/09/2024 11:44 am, Frediano Ziglio wrote:
>     > The various filters are removing all optimisations.
>     > No need to have all optimisations turned off.
>     >
>     > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
>     > ---
>     >  xen/arch/x86/boot/Makefile | 1 +
>     >  1 file changed, 1 insertion(+)
>     > ---
>     > Changes since v1
>     > - reuse optimization level from XEN_CFLAGS.
>     >
>     > diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
>     > index 8f5bbff0cc..8352ce023b 100644
>     > --- a/xen/arch/x86/boot/Makefile
>     > +++ b/xen/arch/x86/boot/Makefile
>     > @@ -16,6 +16,7 @@ $(call
>     cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
>     >  CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
>     >  CFLAGS_x86_32 += -nostdinc -include $(filter
>     %/include/xen/config.h,$(XEN_CFLAGS))
>     >  CFLAGS_x86_32 += $(filter -I%,$(XEN_CFLAGS))
>     > +CFLAGS_x86_32 += $(filter -O%,$(XEN_CFLAGS))
>
>     I'm pretty sure this can be part of the prior expression,
>
>     CFLAGS_x86_32 += $(filter -I% -O%,$(XEN_CFLAGS))
>
>     Happy to fix on commit.
>
>
>
> No objections.

Ok.  Will get that sorted.

>
> On a similar subject we don't pass -D__XEN__ to these files, this
> looks wrong to me, they are Xen sources.
> Should I send a separate patch after this with that change?

Yeah, we should be retaining that.  I can fold it in too, with a minor
adjustment to the commit message.

~Andrew