Re: [PATCH v2] arm64: Allow CONFIG_AUTOFDO_CLANG to be selected

[Yabin Cui]

 On Mon, Dec 9, 2024 at 10:56 AM Will Deacon <will@xxxxxxxxxx> wrote:
>
> (Aside: please try to avoid top-posting on the public lists as it messes up
> the flow of conversation; I'll try to piece this back together.)
>
> On Mon, Dec 09, 2024 at 09:30:50AM -0800, Rong Xu wrote:
> > On Mon, Dec 9, 2024 at 8:20 AM Will Deacon <will@xxxxxxxxxx> wrote:
> > > On Mon, Nov 18, 2024 at 02:25:40PM -0800, Yabin Cui wrote:
> > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > index fd9df6dcc593..c3814df5e391 100644
> > > > --- a/arch/arm64/Kconfig
> > > > +++ b/arch/arm64/Kconfig
> > > > @@ -103,6 +103,7 @@ config ARM64
> > > >       select ARCH_SUPPORTS_PER_VMA_LOCK
> > > >       select ARCH_SUPPORTS_HUGE_PFNMAP if TRANSPARENT_HUGEPAGE
> > > >       select ARCH_SUPPORTS_RT
> > > > +     select ARCH_SUPPORTS_AUTOFDO_CLANG
> > > >       select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
> > > >       select ARCH_WANT_COMPAT_IPC_PARSE_VERSION if COMPAT
> > > >       select ARCH_WANT_DEFAULT_BPF_JIT
> > >
> > > After this change, both arm64 and x86 select this option unconditionally
> > > and with no apparent support code being added. So what is actually
> > > required in order to select ARCH_SUPPORTS_AUTOFDO_CLANG and why isn't
> > > it just available for all architectures instead?

I think it's similar to ARCH_SUPPORTS_LTO_CLANG, which also doesn't need any
support code but requires testing to ensure it works on a specific architecture.

>
> > Enabling an AutoFDO build requires users to explicitly set CONFIG_AUTOFDO_CLANG.
> > The support code is in Commit 315ad8780a129e82 (kbuild: Add AutoFDO
> > support for Clang build).
>
> Yes, that is precisely my point. The user has to enable
> CONFIG_AUTOFDO_CLANG anyway, so what is the point in having
> ARCH_SUPPORTS_AUTOFDO_CLANG. Why would an architecture _not_ want to
> select that?
>
> > We are not enabling this for all architectures because AutoFDO's optimized build
> > relies on Last Branch Records (LBR) which aren't available on all architectures.
>
> So? ETM isn't available on all arm64 machines and I doubt whether LBR is
> available on _all_ x86 machines either. So there's a runtime failure
> mode that needs to be handled anyway and I don't think the arch-specific
> Kconfig option is really doing anything useful.

My understanding of the benefits of ARCH_SUPPORTS_AUTOFDO_CLANG is:
1. Generally, we don't prefer to collect an AutoFDO profile on one
architecture and use it to build the kernel for another architecture.
This is because the profile misses data for architecture-dependent
code. ARCH_SUPPORTS_AUTOFDO_CLANG can partially prevent this from
happening.

2. Building a kernel with an AutoFDO profile involves using new
optimization flags for clang.  Having ARCH_SUPPORTS_AUTOFDO_CLANG=y
for one architecture means someone has tested building a kernel with
an AutoFDO profile on this architecture.
>
> Will