[PATCH v2 02/41] Makefile: Allow LTO to be disabled for a build

Thu Nov 4 17:44:36 CET 2021

On Thu, Nov 04, 2021 at 09:12:02AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 4 Nov 2021 at 08:55, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Wed, Nov 03, 2021 at 08:49:01PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 1 Nov 2021 at 15:58, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Sun, Oct 31, 2021 at 05:46:43PM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Wed, 27 Oct 2021 at 07:13, Tom Rini <trini at konsulko.com> wrote:
> > > > > >
> > > > > > On Wed, Oct 27, 2021 at 02:21:17PM +0200, Heinrich Schuchardt wrote:
> > > > > > >
> > > > > > >
> > > > > > > On 10/27/21 10:50, Ilias Apalodimas wrote:
> > > > > > > > Hi Simon
> > > > > > > >
> > > > > > > > How does this patch related to the standard boot series? Shouldn't
> > > > > > > > this be a completely separate patch?
> > > > > > > >
> > > > > > > > Thanks
> > > > > > > > /Ilias
> > > > > > > >
> > > > > > > > On Sun, 24 Oct 2021 at 02:26, Simon Glass <sjg at chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > > > > > > > significantly reduce the size of U-Boot binaries. So far it has been
> > > > > > > > > made available for selected ARM boards and sandbox.
> > > > > > > > >
> > > > > > > > > However, incremental builds are much slower when LTO is used. For example,
> > > > > > > > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > > > > > > > seconds with LTO enabled.
> > > > > > > > >
> > > > > > > > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > > > > > > > development if needed, for faster builds.
> > > > > > > > >
> > > > > > > > > Add some documentation about LTO while we are here.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > (no changes since v1)
> > > > > > > > >
> > > > > > > > >   Makefile                           | 18 +++++++++++++-----
> > > > > > > > >   arch/arm/config.mk                 |  4 ++--
> > > > > > > > >   arch/arm/include/asm/global_data.h |  2 +-
> > > > > > > > >   doc/build/gcc.rst                  | 17 +++++++++++++++++
> > > > > > > > >   4 files changed, 33 insertions(+), 8 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > > > index b79b2319ff6..7057723e046 100644
> > > > > > > > > --- a/Makefile
> > > > > > > > > +++ b/Makefile
> > > > > > > > > @@ -434,6 +434,9 @@ KBUILD_CFLAGS       += -fshort-wchar -fno-strict-aliasing
> > > > > > > > >   KBUILD_AFLAGS   := -D__ASSEMBLY__
> > > > > > > > >   KBUILD_LDFLAGS  :=
> > > > > > > > >
> > > > > > > > > +# Set this to "n" use of LTO for this build, e.g. LTO_BUILD=n
> > > > > > > > > +LTO_BUILD      ?= y
> > > > > > >
> > > > > > > This does not allow LTO_BUILD=y to enable LTO for CONFIG_LTO=n.
> > > > > >
> > > > > > I don't understand why we need this patch at all.  If you want to
> > > > > > disable LTO, disable LTO.  Yes, LTO makes linking take longer which can
> > > > > > be annoying on iterative development.  I have a few different "HACK: DO
> > > > > > NOT PUSH:  ..." things I git am at the start of a branch, depending on
> > > > > > needs.  You can just do that to drop "imply LTO" from the SANDBOX stanza
> > > > > > in arch/Kconfig.  We do not need a whole thing around a CONFIG option
> > > > > > that can be disabled in the defconfig, or local .config file even.
> > > > > >
> > > > >
> > > > > Cranky time.
> > > > >
> > > > > Of course we don't *need* it. I could just buy a slower build machine
> > > > > and type with two fingers. There are lots of ways to slow things down
> > > > > and LTO is one of them. I change branches at least a dozen times a day
> > > > > and am always trying things out from patchwork. I am sure others do
> > > > > too. LTO dramatically slows down builds. Having a way to easily do
> > > > > this from the build system saves time.
> > > >
> > > > Maybe the answer is that LTO just isn't appropriate for sandbox.  We're
> > > > not doing any specific tests for LTO anywhere (nor does that seem
> > > > appropriate), and we do have platforms in CI that run tests other than
> > > > building, with LTO.
> > >
> > > It has value as a test, I presume, and a demo of how it works. Also it
> > > runs most of the tests.
> > >
> > > But I'm happy to disable it if that helps.
> > >
> > > Still, it doesn't really solve the issue. The same thing happens when
> > > building real boards.
> >
> > Well, a big part of the problem here is I strongly disagree with a
> > make-line flag to override a CONFIG option.  I also hear your use case
> > of "I build this platform so frequently per development session it's a
> > noticeable slowdown and a 'LOCAL:' commit will also screw up my
> > workflow".  So where do we go?  LTO is a size versus speed trade-off
> > (and -ffunction-sections/-fdata-sections/--gc-unused is a much smaller
> > speed trade-off) that's more important on real hardware (and also not
> > used as often as it might be, at least so far).  On real boards, there's
> > less of a "just turn it off" option because it's required to be small
> > enough to use on the hardware.  Since that's not the case on sandbox,
> > maybe we just need to turn it on in more QEMU platforms (to get broader
> > test coverage in CI) and off in sandbox.
> 
> This is all true...and certainly sandbox would solve the main issue I
> have. If we turn it off for sandbox we should perhaps have CI turn it
> on...
> 
> It is similar to the dtc thing, where we have a DTC variable to avoid
> pointlessly building dtc. I guess you are worried about having too
> many of these sorts of things?

Well, the DTC thing to me is at least a case of "external tool exists,
stop building it when it's already provided, versioning tools like this
is bad".  LTO is a thing that impact the build itself and can (so long
as it fits in hardware still) be enabled/disabled by the user.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20211104/11f7913c/attachment.sig>