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