diff mbox series

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

Message ID 20240909104402.67141-1-frediano.ziglio@cloud.com (mailing list archive)
State Superseded
Headers show
Series [v2] x86/boot: Optimise 32 bit C source code | expand

Commit Message

Frediano Ziglio Sept. 9, 2024, 10:44 a.m. UTC
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.

Comments

Andrew Cooper Sept. 9, 2024, 10:47 a.m. UTC | #1
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
Frediano Ziglio Sept. 9, 2024, 10:51 a.m. UTC | #2
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
Andrew Cooper Sept. 9, 2024, 10:54 a.m. UTC | #3
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
diff mbox series

Patch

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 :=