Page MenuHomeFreeBSD

Use -Os when compiling all bootloaders, not just some
AbandonedPublic

Authored by ngie on Feb 3 2017, 8:55 PM.

Details

Reviewers
imp
tsoome
Summary

Use -Os when compiling all bootloaders, not just some

This reduces the sizes of all bootloaders where -Os wasn't explicitly set in
CFLAGS, e.g. gptzfsboot and pxeboot

It's a part of Isilon's patch to reduce the size of gptboot, but set globally

zfsboot is unchanged size-wise because the bootloader size is set at 128kB,
per sys/boot/i386/zfsboot/Makefile@r304321 .

MFC after: 1 month
Sponsored by: Dell EMC Isilon

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7213
Build 7383: arc lint + arc unit

Event Timeline

ngie retitled this revision from to Use -Os when compiling all bootloaders, not just some.
ngie updated this object.
ngie edited the test plan for this revision. (Show Details)
ngie added reviewers: imp, tsoome.
ngie added subscribers: dim, markj.

In the past we've only used this where it makes sense.
That's because in the past -Os has often resulted in either mis-compiled code or has resulted in larger binaries than w/o it.
I suppose it is OK, but I'm leery since this touches every single architecture.

Check also D7600 - I did try to take on -Os there, but it is a bit mixed up with crypto stuff. Maybe some ideas are useful in your work.

In D9430#194891, @imp wrote:

In the past we've only used this where it makes sense.
That's because in the past -Os has often resulted in either mis-compiled code or has resulted in larger binaries than w/o it.
I suppose it is OK, but I'm leery since this touches every single architecture.

That is good point as well.

Having thought about it, I'd go farther: absolutely do not commit this change unless you can test every single boot loader that this change touches. So does it work on mips? does it work on powerpc? Do these binaries you made actually work?

In D9430#194891, @imp wrote:

In the past we've only used this where it makes sense.
That's because in the past -Os has often resulted in either mis-compiled code or has resulted in larger binaries than w/o it.
I suppose it is OK, but I'm leery since this touches every single architecture.

-Os miscompiling binaries is a potential issue, but if that was the case that's a compiler bug to care about and fix.

In D9430#194902, @imp wrote:

Having thought about it, I'd go farther: absolutely do not commit this change unless you can test every single boot loader that this change touches. So does it work on mips? does it work on powerpc? Do these binaries you made actually work?

If only I had working QEMU emulators to play with...

In D9430#194903, @ngie wrote:
In D9430#194891, @imp wrote:

In the past we've only used this where it makes sense.
That's because in the past -Os has often resulted in either mis-compiled code or has resulted in larger binaries than w/o it.
I suppose it is OK, but I'm leery since this touches every single architecture.

-Os miscompiling binaries is a potential issue, but if that was the case that's a compiler bug to care about and fix.

While that's true, that's no reason to purposely break something in the tree.

In D9430#194904, @ngie wrote:
In D9430#194902, @imp wrote:

Having thought about it, I'd go farther: absolutely do not commit this change unless you can test every single boot loader that this change touches. So does it work on mips? does it work on powerpc? Do these binaries you made actually work?

If only I had working QEMU emulators to play with...

Ask for help from others. There's several folks that have qemu environments that work for all our ports except maybe sparc64.

In D9430#194914, @imp wrote:

...

Ask for help from others. There's several folks that have qemu environments that work for all our ports except maybe sparc64.

Yeah, I'll try to make this work again, and if successful document it.

In D9430#194891, @imp wrote:

In the past we've only used this where it makes sense.
That's because in the past -Os has often resulted in either mis-compiled code or has resulted in larger binaries than w/o it.
I suppose it is OK, but I'm leery since this touches every single architecture.

That is good point as well.

In D9430#194915, @ngie wrote:
In D9430#194914, @imp wrote:

...

Ask for help from others. There's several folks that have qemu environments that work for all our ports except maybe sparc64.

Yeah, I'll try to make this work again, and if successful document it.

Still, even if we just build everything with -Os, actually even x86 bits are built without. I did simple test - built ficl, geli, zfs, libstand and libi386 with -Os

-rw-r--r-- 1 root wheel 368640 Feb 4 12:24 zfsloader
-r-xr-xr-x 1 root wheel 417792 Feb 1 14:34 /boot/zfsloader

ngie edited edge metadata.

Add ps3 bootloader changes to get it to compile with -Os

sys/boot/powerpc/ps3/Makefile
14

You might want to see if there's any of the compiler support routines currently included that become unused if you compile -Os.

It might be a good idea to document this, but I have mixed feelings about that since what happens with -Os changes from compiler release to release as well as between compilers.

In D9430#194902, @imp wrote:

Having thought about it, I'd go farther: absolutely do not commit this change unless you can test every single boot loader that this change touches. So does it work on mips? does it work on powerpc? Do these binaries you made actually work?

I don't have a working example to go off of for MIPS, but I was able to boot a cd image on ppc64 and USB image on arm64 using qemu.

  • Couldn't verify ppc because qemu support is busted.
  • Couldn't verify mips* because I lack a qemu recipe that uses PXE/NFS roots and release/ doesn't support mips*.
  • I'll try sparc64 in a bit..
sys/boot/powerpc/ps3/Makefile
14

I'm using WITH_SYSTEM_COMPILER and compiling ^/head on ^/stable/11 . I wonder if that had something to do with this.

Also, this comes from libgcc // libcompiler-rt.

Ask on #bsdmips for help with MIPS. Adrian and Baldwin should be able to help. Might be able to get direction from Robert Watson or Brooks Davis since they do something with some VM for their CHERRI work.

emaste added inline comments.
sys/boot/Makefile.inc
8

I wonder if we should use -Oz for Clang?

sys/boot/Makefile.inc
8

It has to be verified for sure, last time I did test, the gptzfsboot was broken by it - but there has been some clang updates meanwhile.

ngie added a subscriber: bdrewery.

Abandoning revision since I will not be working on the build system proper anymore, apart from test integration or any components that directly correlate with testing.

Bryan/Mark can work on upstreaming Isilon's equivalent change here (which just affected gptboot, not the other boot loaders).