Re: clang-nightly: vdso/compat_gettimeofday.h:152:15: error: instruction variant requires ARMv6 or later

[Sylvestre Ledru]

Le 05/12/2023 à 16:04, Nathan Chancellor a écrit :
> On Tue, Dec 05, 2023 at 07:34:40AM +0100, Arnd Bergmann wrote:
> > On Mon, Dec 4, 2023, at 23:33, Nathan Chancellor wrote:
> > > On Mon, Dec 04, 2023 at 11:13:04AM -0700, Nathan Chancellor wrote:
> > >
> > > > I am still investigating into what (if anything) can be done to resolve
> > > > this on the kernel side. We could potentially revert commit
> > > > ddc72c9659b5 ("kbuild: clang: do not use CROSS_COMPILE for target
> > > > triple") but I am not sure that will save us from that change, as
> > > > tuxmake's CROSS_COMPILE=arm-linux-gnueabihf will cause us to have an
> > > > armv7 CPU even though we may not be building for armv7.
> > >
> > > Okay, this is a pretty awful situation the more I look into it :(
> > >
> > > The arm64 compat vDSO build is easy enough to fix because we require use
> > > of the integrated assembler, which means we can add '-mcpu=generic' (the
> > > default in LLVM for those files based on my debugging) to those files
> > > and be done with it:
> > >
> > > diff --git a/arch/arm64/kernel/vdso32/Makefile
> > > b/arch/arm64/kernel/vdso32/Makefile
> > > index 1f911a76c5af..5f5cb722cfc2 100644
> > > --- a/arch/arm64/kernel/vdso32/Makefile
> > > +++ b/arch/arm64/kernel/vdso32/Makefile
> > > @@ -9,6 +9,10 @@ include $(srctree)/lib/vdso/Makefile
> > >   ifeq ($(CONFIG_CC_IS_CLANG), y)
> > >   CC_COMPAT ?= $(CC)
> > >   CC_COMPAT += --target=arm-linux-gnueabi
> > > +# Some distributions (such as Debian) change the default CPU for the
> > > +# arm-linux-gnueabi target triple, which can break the build.
> > > Explicitly set
> > > +# the CPU to generic, which is the default for Linux in LLVM.
> > > +CC_COMPAT += -mcpu=generic
> > >   else
> > >   CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
> > >   endif
> >
> > I'm still trying to follow what is actually going on. I
> > see that we pass
> >
> > VDSO_CAFLAGS += -march=armv8-a
> >
> > which is meant to tell the compiler that we want it to
> > use ARMv8 compatible instructions. Is the problem that
> > clang ignores this flag, or do we not pass it correctly?
> >
> > I would have expected -march=armv8-a to be better than
> > -mcpu=generic here, as it allows the compiler to use
> > a wider set of instructions that is still guaranteed to
> > be available on everything it will run on.
>
> I should have made it clearer in that message that adding
> '-mcpu=generic' was only to avoid the logic added by that Debian LLVM
> change, not because I believe the kernel is doing something incorrectly
> now. From what I could tell following through LLVM's code, '-march='
> determines the default CPU, which is then used to further inform the
> full target triple and by overriding the CPU where that patch did, it
> was just blowing away the user's request. By providing an '-mcpu='
> option explicitly, it would avoid the default selection logic and we
> would get what we asked for.
>
> > > Sylvestre, I strongly believe you should consider reverting that change
> > > or give us some compiler flag that allows us to fallback to upstream
> > > LLVM's default CPU selection logic. I think that hardcoding Debian's
> > > architecture defintions based on the target triple into the compiler
> > > could cause issues for other projects as well. For example,
> > > '--target=arm-linux-gnueabi -march=armv7-a' won't actually target ARMv7:
> > >
> > >    $ echo 'int main(void) { asm("dsb"); return 0; }' | \
> > >          clang --target=arm-linux-gnueabi -march=armv7-a \
> > >          -x c -c -o /dev/null -v -
> > >    ...
> > >     "/usr/bin/clang-17" -cc1 -triple armv7-unknown-linux-gnueabi ...
> > >    ...
> > >
> > > vs.
> > >
> > >    $ echo 'int main(void) { asm("dsb"); return 0; }' | \
> > >          clang --target=arm-linux-gnueabi -march=armv7-a \
> > >          -x c -c -o /dev/null -v -
> > >    ...
> > >    "<prefix>/bin/clang-18" -cc1 -triple armv5e-unknown-linux-gnueabi ...
> > >    ...
> >
> > Right, the kernel definitely relies on -march= taking
> > precedence over the default CPU, the same way that we
> > tell the compiler to pick a non-default endianess or ABI.
>
> Agreed, I have yet to test the new version of the patch but I see you
> and Ard have given input on it, so hopefully it does not have any
> problems like this.

Matthias, as cc, pushed a potential fix for debian/ubuntu packages!
https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/commit/01a06b481e5a2610c7387149b58978c3ec281f2c

Cheers,
Sylvestre