All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Rong Xu <xur@google.com>
Cc: Yonghong Song <yonghong.song@linux.dev>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Nicolas Schier <nicolas.schier@linux.dev>,
	Nick Desaulniers <nick.desaulniers+lkml@gmail.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	Miguel Ojeda <ojeda@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Alice Ryhl <aliceryhl@google.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	"Mike Rapoport (Microsoft)" <rppt@kernel.org>,
	Rafael Aquini <aquini@redhat.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Stafford Horne <shorne@gmail.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Piotr Gorski <piotrgorski@cachyos.org>,
	Venkat Rao Bagalkote <venkat88@linux.ibm.com>,
	Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
	Teresa Johnson <tejohnson@google.com>,
	linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [PATCH v9 3/3] kbuild: distributed build support for Clang ThinLTO
Date: Fri, 22 May 2026 20:55:52 -0700	[thread overview]
Message-ID: <20260523035552.GA799545@ax162> (raw)
In-Reply-To: <CAF1bQ=ScvbDL4TY8=FJzwULP9KS-aZ4K=o4f+QxGssca+Q7ovQ@mail.gmail.com>

On Fri, May 22, 2026 at 08:29:53PM -0700, Rong Xu wrote:
> Thanks Nathan for comments and suggestions. My comments are inlined.
> 
> On Fri, May 22, 2026 at 6:17 PM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > On Fri, May 22, 2026 at 11:58:35AM -0700, Rong Xu wrote:
> > > On Fri, May 22, 2026 at 11:44 AM Yonghong Song <yonghong.song@linux.dev> wrote:
> > > > On 5/22/26 11:14 AM, Rong Xu wrote:
> > > > > On Fri, May 22, 2026 at 10:52 AM Yonghong Song <yonghong.song@linux.dev> wrote:
> > > > >> On 5/22/26 8:32 AM, Rong Xu wrote:
> > > > >>> On Thu, May 21, 2026 at 2:57 PM Yonghong Song <yonghong.song@linux.dev> wrote:
> > > > >>>>
> > > > >>>> On 5/21/26 11:31 AM, Rong Xu wrote:
> > > > >>>>> Yonghong, thanks for the update.
> > > > >>>>>
> > > > >>>>> Regarding this guard: ther is a period of Clang (before this patch and
> > > > >>>>> after your first patch), even though ld.lld having these options
> > > > >>>>> (specifically --lto-whole-program-visibility -mllvm
> > > > >>>>> -always-rename-promoted-locals=false), distributed ThinLTO mode
> > > > >>>>> remains unsupported, correct? What the behvior of using this options
> > > > >>>>> in distributed mode with these compilers? nop or it will lead to
> > > > >>>>> error?
> > > > >>>> The in-process thin-lto support is landed on Feb 27.
> > > > >>>> The distributed thin-lto support is landed on Apr 24.
> > > > >>>>
> > > > >>>> If people are using distributed thin-lto in kernel between Feb 27 and
> > > > >>>> Apr 24, there will be some issues. But people typically use released
> > > > >>>> compiler, so we should be fine.
> > > > >>> This is not the case for us (google). We do use compiler b/w releases,
> > > > >>> and we build our own.
> > > > >>>
> > > > >>> What is the issue if we use the compiler in b/w Feb27 and Apr24?
> > > > >> If you use the custom compiler between Feb27 and Apr24 and your kernel
> > > > >> will do distributed thin-lto, you can remove
> > > > >>      $(call ld-option,--lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false)
> > > > >> from your kernel. Since you have custom compiler, you can
> > > > >> do some customization for your kernel as well.
> > > > > I am referring to this specific patch -- there are cases that use
> > > > > custom compilers built between the February 27 and April 24 LLVM
> > > > > releases.
> > > > > I don't want to see any breakage for distributed ThinLTO in these cases.
> > > > >
> > > > > I would prefer to keep this guard for distributed ThinLTO for the time
> > > > > being and remove it later. What do you think?
> > > >
> > > > For 'remove it later', when this will happen? When llvm23 cuts the rc
> > > > in July or cut the release in September or the end of bug fix say in December?
> > >
> > > I can remove the guard when the minimal clang containis the 4/24 patch.
> >
> > I think we could just change
> >
> >   ifdef CONFIG_LTO_CLANG_THIN
> >   KBUILD_LDFLAGS += $(call ld-option,--lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false)
> >   endif
> >
> > to
> >
> >   ifneq ($(CONFIG_LTO_CLANG_THIN)$(CONFIG_LTO_CLANG_THIN_DIST),)
> >   KBUILD_LDFLAGS += $(if $(call clang-min-version,230100),--lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false)
> >   endif
> >
> > when LLVM 23.1.0-rc1 is out to avoid that Feb 27 to Apr 24 breakage?
> 
> I don't understand why we need this new "ifneq ..." guard. We have
> already checked CONFIG_LTO_CLANG=y, and CONFIG_LTO_CLANG_FULL != y.
> I think this "ifnef ..." will always be true (so redundant).

Ah, yeah, I missed since I did not look at the full context. That could
just be

  ifneq ($(call clang-min-version,230100),)
  KBUILD_LDFLAGS += --lto-whole-program-visibility -mllvm -always-rename-promoted-locals=false
  endif

then.

> >
> > > > If we carry the guard (for distributed thinlto) in this kernel release,
> > > > that means at some point, we will need to do kernel backport, extra work.
> > >
> > > I don't understand here: this is part of the distributed thinlto patch
> > > that you would need merge anyay.
> > > where is the extra work for backporting?
> > >
> > > > Also, since you are building custom in-development compiler, you can
> > > > disable this flag -always-rename-promoted-locals, problem solved, right?
> > >
> > > I'm not only talking about me. There are other users also use this way.
> > > BTW, even in google, I don't control the compiler that being used.
> >
> > So in general, we assume that folks who are using prerelease compilers
> > (i.e., 23.0.0 in this case) are upgrading them regularly, as we may need
> > to workaround or fix issues that happen in main that cannot be
> > dynamically detected (at least not easily or conveniently). For example,
> > recent main versions of clang have support for -Wattribute-alias, so we
> > need to turn it off via #pragmas like done for GCC, which will break
> > things for clang-23 versions that do not have -Wattribute-alias:
> >
> >   https://git.kernel.org/nathan/c/bc5ffe737f56ee2734597069ed71f48830549483
> >
> > So the argument about breaking compilers in between Feb 27 and Apr 24 is
> > not really relevant in my opinion since they should be short lived in
> > terms of deployment. However, given that things work the way the check
> > is currently written and the LLVM 23 branch is only a couple of months
> > away, I am fine with just sticking with how it is currently written and
> > updating it when things are more guaranteed to work.
> 
> This works for me. I can add a comment here.
> Like the following:
> =====
>  ifdef CONFIG_LTO_CLANG
> -ifdef CONFIG_LTO_CLANG_THIN
> +ifdef CONFIG_LTO_CLANG_FULL
> +CC_FLAGS_LTO   := -flto
> +else
>  CC_FLAGS_LTO   := -flto=thin -fsplit-lto-unit
> +
> +# The following clang options added on 2026-02-27 lack distributed
> +# ThinLTO support until the 2026-04-24. Disabling for distributed
> +# builds until the minimum clang version is updated.

I think the dates are just noise. I think we can keep it short and sweet
with something like:

  # These LLVM options were initially added with only in-process ThinLTO
  # support, so avoid distributed ThinLTO support for now.

Otherwise, seems fine to me. I will set a reminder to revisit this when
LLVM 23 has branched so we can just turn it into a version check.

> +ifdef CONFIG_LTO_CLANG_THIN
>  KBUILD_LDFLAGS += $(call ld-option,--lto-whole-program-visibility
> -mllvm -always-rename-promoted-locals=false)
> -else
> -CC_FLAGS_LTO   := -flto
> +endif
>  endif
>  CC_FLAGS_LTO   += -fvisibility=hidden
> =====
> 
> If you are fine with this, I can send v10 for review.
> 
> >
> > --
> > Cheers,
> > Nathan

-- 
Cheers,
Nathan

      reply	other threads:[~2026-05-23  3:55 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-31 15:48 [PATCH v9 0/3] kbuild: distributed build support for Clang ThinLTO xur
2026-03-31 15:48 ` [PATCH v9 1/3] kbuild: move vmlinux.a build rule to scripts/Makefile.vmlinux_a xur
2026-03-31 15:48 ` [PATCH v9 2/3] kbuild: change --thin back to 'T' in $(AR) xur
2026-03-31 15:48 ` [PATCH v9 3/3] kbuild: distributed build support for Clang ThinLTO xur
2026-03-31 16:27   ` Nathan Chancellor
2026-05-21 17:57     ` Yonghong Song
2026-05-21 18:31       ` Rong Xu
2026-05-21 21:56         ` Yonghong Song
2026-05-22 15:32           ` Rong Xu
2026-05-22 17:51             ` Yonghong Song
2026-05-22 18:14               ` Rong Xu
2026-05-22 18:44                 ` Yonghong Song
2026-05-22 18:58                   ` Rong Xu
2026-05-23  1:17                     ` Nathan Chancellor
2026-05-23  3:29                       ` Rong Xu
2026-05-23  3:55                         ` Nathan Chancellor [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260523035552.GA799545@ax162 \
    --to=nathan@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=aquini@redhat.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=justinstitt@google.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=masahiroy@kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=morbo@google.com \
    --cc=mpe@ellerman.id.au \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=nicolas.schier@linux.dev \
    --cc=ojeda@kernel.org \
    --cc=piotrgorski@cachyos.org \
    --cc=rppt@kernel.org \
    --cc=samitolvanen@google.com \
    --cc=shorne@gmail.com \
    --cc=tejohnson@google.com \
    --cc=tglx@linutronix.de \
    --cc=venkat88@linux.ibm.com \
    --cc=xur@google.com \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.