Re: [PATCH] x86: unify x86 Makefile(s)

[Sam Ravnborg <sam@ravnborg.org>]

On Sat, Dec 29, 2007 at 03:22:35AM +0100, Andi Kleen wrote:
> On Friday 28 December 2007 22:23:41 Sam Ravnborg wrote:
> > - For 64 bit the sub architecture stuff is not used but structure
> >  is kept to make it easy to introduce.
> 
> I hope not. subarch is one of the main disaster areas in the i386 port.
The Makefile part looks reasonably ok - the implementation I have not looked at.

> 
> > +        # gcc doesn't support -march=core2 yet as of gcc 4.3, but I hope it
> > +        # will eventually. Use -mtune=generic as fallback
> 
> The comment is out of date, 4.3 has a -march=core2 now.
OK
 
> 
> > +        cflags-$(CONFIG_MCORE2) += \
> > +                $(call cc-option,-march=core2,$(call cc-option,-mtune=generic))
> > +        cflags-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=generic)
> > +        KBUILD_CFLAGS += $(cflags-y)
> > +
> > +        KBUILD_CFLAGS += -mno-red-zone
> > +        KBUILD_CFLAGS += -mcmodel=kernel
> > +
> > +        KBUILD_CFLAGS += -Wno-sign-compare
> 
> That one should be on for i386 too. It was a workaround for a gcc prelease
> that unfortunately was shipped in a suse release.
OK
> 
> > +        KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> 
> And that too should be on for i386 too.
OK
> 
> > +        ifneq ($(CONFIG_DEBUG_INFO),y)
> > +                # -fweb shrinks the kernel a bit, but the difference is very small
> > +                # it also messes up debugging, so don't use it for now.
> > +                #KBUILD_CFLAGS += $(call cc-option,-fweb)
> > +        endif
> 
> Can be dropped completely now.
OK - I was tempted to do so as it is a noop anyway.

> 
> > +        # -funit-at-a-time shrinks the kernel .text considerably
> > +        # unfortunately it makes reading oopses harder.
> > +        KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
> 
> Should be on for i386 too (see other mail)
OK
> > +
> > +        # prevent gcc from generating any FP code by mistake
> > +        KBUILD_CFLAGS += $(call cc-option,-mno-sse -mno-mmx -mno-sse2 -mno-3dnow,)
> 
> Should be on for i386 too (in fact i thought it was already, surprised) 
OK
> 
> > +        # this works around some issues with generating unwind tables in older gccs
> > +        # newer gccs do it by default
> > +        KBUILD_CFLAGS += -maccumulate-outgoing-args
> 
> That is only needed for the unwinder; I readd it on i386 too in the out of tree
> unwinder patch. Strictly without it it's not needed, although it can be useful
> for kgdb or fireproxy or crash for the gdb unwinder. But that applies to i386 too.
> So probably it should be either dropped on both or readded to i386. I would
> be for readding. iirc it was dropped because it increased code size slightly,
> but code size is not everything. debuggability is more important.

Thanks for the feedback Andi.
I will prepare a small patch serie so we have the additional options applied for
i386 in individual patches and the original patch will only be slightly updated.

	Sam