From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF727C433EF for ; Thu, 4 Nov 2021 15:13:01 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 41DE9611EE for ; Thu, 4 Nov 2021 15:13:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 41DE9611EE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 81A8A836F2; Thu, 4 Nov 2021 16:12:37 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="mM9fdfoG"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9269D83698; Thu, 4 Nov 2021 16:12:26 +0100 (CET) Received: from mail-ua1-x92a.google.com (mail-ua1-x92a.google.com [IPv6:2607:f8b0:4864:20::92a]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 75201836CF for ; Thu, 4 Nov 2021 16:12:17 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-ua1-x92a.google.com with SMTP id o26so11438857uab.5 for ; Thu, 04 Nov 2021 08:12:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=H2fxNRFv75XXAIBloaXCryTBnMoX6mfXYRNy0tAV9KM=; b=mM9fdfoGNynqFCrHOnJGyHAghvmXydcF3HNiBOnRo6gPhF/32RB/h0wJy2UaIrPnbv c+fkaVXUPBVwQc0PyIkQ0uegznj7BFGFW+nki8CgYRJPnG0BXOyDNFoeCw60K+9Uz0RJ djxApNDDOCQkbo1Cjqql8z7tnhVQXbX0srNZo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=H2fxNRFv75XXAIBloaXCryTBnMoX6mfXYRNy0tAV9KM=; b=z7Gf1TTm0inlfNX1h6mJluwUGiiDT5unAnuWIynxDGl+xvK7Jst8/scOWNzKEVUwxp Ytk2ZVG6Ffy0S0roqwt9mGf+XuX1JAIrblHlc4EhrjukYdwXWtnvIpmZOFyKvJQYfVZv mXZ3wVzFzGUX52c7pqgF74dvOiCfa/0XU9nW7RlFzdQBqZK7M7QJxTaORS+JbDIf0RS0 zHp2egKM6UNTvhEqlfNvf6JTdivWz76x1XGja7S0N7IIdT3J6+j57VaZ9tL3XEudO2gp OxZ2RsrtgWg3XkU2aWVpNAU69vyrLHmXffPc+nLy+ScQLWuS4WKrKn0ExvZk6aUMbpNP apYw== X-Gm-Message-State: AOAM531mvH4Y8fvs1XXj7CDAP6Kt/3ngc1I2ISo97cr0iYTOJhHRtQIL cvGV5K7GHpMIVMaozweY1rFVBEREv41iiZ6zNC1NOg== X-Google-Smtp-Source: ABdhPJwQ6rugYT9Brk1Jwl3HPDzC8Mj5XekgX22gFXU2BLqtKU9434QxT1FsjV84xKEgRZW4OOXqDhfP4pEX5fc5zJA= X-Received: by 2002:a67:c308:: with SMTP id r8mr34833773vsj.20.1636038734172; Thu, 04 Nov 2021 08:12:14 -0700 (PDT) MIME-Version: 1.0 References: <20211023232635.9195-1-sjg@chromium.org> <20211023232635.9195-3-sjg@chromium.org> <20211027131340.GT8284@bill-the-cat> <20211101215805.GN24579@bill-the-cat> <20211104145525.GM24579@bill-the-cat> In-Reply-To: <20211104145525.GM24579@bill-the-cat> From: Simon Glass Date: Thu, 4 Nov 2021 09:12:02 -0600 Message-ID: Subject: Re: [PATCH v2 02/41] Makefile: Allow LTO to be disabled for a build To: Tom Rini Cc: Heinrich Schuchardt , U-Boot Mailing List , Michal Simek , Daniel Schwierzeck , Steffen Jaeckel , =?UTF-8?B?TWFyZWsgQmVow7pu?= , Lukas Auer , Dennis Gilmore , Masahiro Yamada , Ilias Apalodimas , Heinrich Schuchardt Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Tom, On Thu, 4 Nov 2021 at 08:55, Tom Rini 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 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 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 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 > > > > > > > > --- > > > > > > > > > > > > > > > > (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? Regards, SImon