Re: [PATCH] Fix CFLAGS overwrite by Makefile

Hi,

On Wed, May 25, 2011 at 9:43 AM, John Baldwin <jhb_at_freebsd.org> wrote:
> On Tuesday, May 24, 2011 5:30:37 pm Arnaud Lacombe wrote:
>> Hi,
>>
>> On Tue, May 24, 2011 at 4:41 PM, Dimitry Andric <dim_at_freebsd.org> wrote:
>> > On 2011-05-24 22:09, Arnaud Lacombe wrote:
>> >>
>> >> Many Makefile (espectially under sys/boot/) overwrite the value of
> CFLAGS.
>> >> This is an issue if you want to generate code for a specific CPU as
> before
>> >> the
>> >> Makefile is interpreted, CFLAGS might already have been set with CPU
>> >> specific
>> >> settings by<bsd.cpu.mk>, which is source from sys.mk.
>> >
>> > ...
>> >>
>> >> --- a/sys/boot/i386/boot2/Makefile
>> >> +++ b/sys/boot/i386/boot2/Makefile
>> >
>> > ...
>> >
>> > The problem with this patch is that for some of the things you fixed,
>> > stuff like boot-time programs, you NEVER want any CPU specific settings!
>> > You must use the default, lowest common denominator setting instead, or
>> > there is no guarantee the boot program will be correct.
>> >
>> To use your argument against you: with the default, the boot program
>> is not correct (see below).
>
> Thousands of machines succesfully booting FreeBSD would seem to contradict
> your assertion.
>
my assertion is backed by experiment, so I'm fine with the contradiction.

>> > So that is why these Makefiles purposefully overwrite CFLAGS. it is not
>> > by accident.
>> You just might be right, but unless the code say the overwrite is
>> _on_purpose_, I would not assume the state of mind of the author, one
>> way or another.
>
> The boot code is certainly intended to be something that works across the
> board.  Also, I doubt you will see any user-visible performance difference
> from changing the optimization options for the boot code.
>
Saving a few kilobytes of space might be more important than speed in
some environment.

>> > Besides, for space-constrained things like boot2, you
>> > might not even be able to compile it when using non-standard settings,
>> > since the code might grow too large.
>> >
>> or can shrink by using more optimized instructions.
>
> Well, your test in a later e-mail is a bit flawed.  GCC tends to insert
> a lot of padding for newer CPUs to align things on more optimal boundaries.
>
That does not explain at all why the default setting always produces
the worst code, size-wise.

> We run 'sed' over the assembly version of boot2 to strip all that out.
>
... to save 4 bytes.

> However, the more important point for the boot code is that it needs to
> just work.
>
and it does not, because gcc defaults are _bad_.

 - Arnaud