Re: [PATCH v9 3/3] kbuild: distributed build support for Clang ThinLTO

[Nathan Chancellor]

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@xxxxxxxxxx> 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@xxxxxxxxx> wrote:
> > > > On 5/22/26 11:14 AM, Rong Xu wrote:
> > > > > On Fri, May 22, 2026 at 10:52 AM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
> > > > >> On 5/22/26 8:32 AM, Rong Xu wrote:
> > > > >>> On Thu, May 21, 2026 at 2:57 PM Yonghong Song <yonghong.song@xxxxxxxxx> 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