2021-03-04 14:30:30

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] MIPS: Add comment about CONFIG_MIPS32_O32 in loongson3_defconfig when build with Clang

On Thu, Mar 04, 2021 at 09:15:44AM +0800, Tiezhu Yang wrote:
> When build kernel with Clang [1]:
>
> $ make CC=clang loongson3_defconfig
> $ make CC=clang
>
> there exists the following error:
>
> Checking missing-syscalls for O32
> CALL scripts/checksyscalls.sh
> error: ABI 'o32' is not supported on CPU 'mips64r2'
> make[1]: *** [Kbuild:48: missing-syscalls] Error 1
> make: *** [arch/mips/Makefile:419: archprepare] Error 2
>
> This is a known bug [2] with Clang, as Simon Atanasyan said,
> "There is no plan on support O32 for MIPS64 due to lack of
> resources".
>
> It is not a good idea to remove this config due to GCC works
> well, so add comment to point out this bug and suggest the
> users to remove CONFIG_MIPS32_O32=y in defconfig when build
> kernel with Clang.
>
> [1] https://www.kernel.org/doc/html/latest/kbuild/llvm.html
> [2] https://bugs.llvm.org/show_bug.cgi?id=38063
>
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
> arch/mips/configs/loongson3_defconfig | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/mips/configs/loongson3_defconfig b/arch/mips/configs/loongson3_defconfig
> index 0e79f81..cacf9dd 100644
> --- a/arch/mips/configs/loongson3_defconfig
> +++ b/arch/mips/configs/loongson3_defconfig
> @@ -35,6 +35,9 @@ CONFIG_NUMA=y
> CONFIG_SMP=y
> CONFIG_HZ_256=y
> CONFIG_KEXEC=y
> +# Please remove CONFIG_MIPS32_O32=y when build with Clang
> +# due to "ABI 'o32' is not supported on CPU 'mips64r2'",
> +# https://bugs.llvm.org/show_bug.cgi?id=38063
> CONFIG_MIPS32_O32=y
> CONFIG_MIPS32_N32=y
> CONFIG_VIRTUALIZATION=y
> --
> 2.1.0
>

I think this might be a better solution. I know that I personally never
read defconfig files if a build fails.

If CONFIG_MIPS32_O32 is broken with clang and the MIPS backend
maintainer has said that it will not be supported due to lack of
resources, then the config should not even be selectable in my opinion.

Cheers,
Nathan

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index d89efba3d8a4..ed35318a759d 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -3315,6 +3315,8 @@ config SYSVIPC_COMPAT
config MIPS32_O32
bool "Kernel support for o32 binaries"
depends on 64BIT
+ # https://bugs.llvm.org/show_bug.cgi?id=38063
+ depends on $(success,$(CC) $(CLANG_FLAGS) -march=mips64 -o32 -c -x c /dev/null -o /dev/null)
select ARCH_WANT_OLD_COMPAT_IPC
select COMPAT
select MIPS32_COMPAT


2021-03-04 14:42:51

by Tiezhu Yang

[permalink] [raw]
Subject: Re: [PATCH] MIPS: Add comment about CONFIG_MIPS32_O32 in loongson3_defconfig when build with Clang

On 03/04/2021 10:02 AM, Nathan Chancellor wrote:
> On Thu, Mar 04, 2021 at 09:15:44AM +0800, Tiezhu Yang wrote:
>> When build kernel with Clang [1]:
>>
>> $ make CC=clang loongson3_defconfig
>> $ make CC=clang

[snip]

> I think this might be a better solution. I know that I personally never
> read defconfig files if a build fails.
>
> If CONFIG_MIPS32_O32 is broken with clang and the MIPS backend
> maintainer has said that it will not be supported due to lack of
> resources, then the config should not even be selectable in my opinion.
>
> Cheers,
> Nathan
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index d89efba3d8a4..ed35318a759d 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -3315,6 +3315,8 @@ config SYSVIPC_COMPAT
> config MIPS32_O32
> bool "Kernel support for o32 binaries"
> depends on 64BIT
> + # https://bugs.llvm.org/show_bug.cgi?id=38063
> + depends on $(success,$(CC) $(CLANG_FLAGS) -march=mips64 -o32 -c -x c /dev/null -o /dev/null)
> select ARCH_WANT_OLD_COMPAT_IPC
> select COMPAT
> select MIPS32_COMPAT

Hi Nathan,

Thank you very much for your reply and suggestion, maybe the following
change is simple, clear and better? If yes, I will send v2 later.

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 3a38d27..f6ba59f 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -3318,6 +3318,8 @@ config SYSVIPC_COMPAT
config MIPS32_O32
bool "Kernel support for o32 binaries"
depends on 64BIT
+ # https://bugs.llvm.org/show_bug.cgi?id=38063
+ depends on !CC_IS_CLANG
select ARCH_WANT_OLD_COMPAT_IPC
select COMPAT
select MIPS32_COMPAT

Thanks,
Tiezhu

2021-03-04 15:00:46

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] MIPS: Add comment about CONFIG_MIPS32_O32 in loongson3_defconfig when build with Clang

On Thu, Mar 04, 2021 at 11:48:09AM +0800, Tiezhu Yang wrote:
> On 03/04/2021 10:02 AM, Nathan Chancellor wrote:
> > On Thu, Mar 04, 2021 at 09:15:44AM +0800, Tiezhu Yang wrote:
> > > When build kernel with Clang [1]:
> > >
> > > $ make CC=clang loongson3_defconfig
> > > $ make CC=clang
>
> [snip]
>
> > I think this might be a better solution. I know that I personally never
> > read defconfig files if a build fails.
> >
> > If CONFIG_MIPS32_O32 is broken with clang and the MIPS backend
> > maintainer has said that it will not be supported due to lack of
> > resources, then the config should not even be selectable in my opinion.
> >
> > Cheers,
> > Nathan
> >
> > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > index d89efba3d8a4..ed35318a759d 100644
> > --- a/arch/mips/Kconfig
> > +++ b/arch/mips/Kconfig
> > @@ -3315,6 +3315,8 @@ config SYSVIPC_COMPAT
> > config MIPS32_O32
> > bool "Kernel support for o32 binaries"
> > depends on 64BIT
> > + # https://bugs.llvm.org/show_bug.cgi?id=38063
> > + depends on $(success,$(CC) $(CLANG_FLAGS) -march=mips64 -o32 -c -x c /dev/null -o /dev/null)
> > select ARCH_WANT_OLD_COMPAT_IPC
> > select COMPAT
> > select MIPS32_COMPAT
>
> Hi Nathan,
>
> Thank you very much for your reply and suggestion, maybe the following
> change is simple, clear and better? If yes, I will send v2 later.

Hi Tiezhu,

I think that the change is simpler but better is subjective. I tend to
prefer tests like mine so that it is not dependent on someone going "oh
hey, this LLVM bug has been fixed so we can turn this config on!".
Instead, the config will just turn on automatically as soon as that bug
is fixed.

However, in this particular case, it does not seem like that will happen
unless someone steps but there have been times where an independent
party will implement some change that benefits them and nobody notices
for a while. Plus, I periodically grep the tree for CC_IS_CLANG to see
if there are any configuration options that can be re-enabled..

Regardless, if Thomas is happy with the below change, so am I, as it
will allow us to test more 64-bit MIPS configurations. I can add an ack
or review at that point in time.

Cheers,
Nathan

> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 3a38d27..f6ba59f 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -3318,6 +3318,8 @@ config SYSVIPC_COMPAT
> config MIPS32_O32
> bool "Kernel support for o32 binaries"
> depends on 64BIT
> + # https://bugs.llvm.org/show_bug.cgi?id=38063
> + depends on !CC_IS_CLANG
> select ARCH_WANT_OLD_COMPAT_IPC
> select COMPAT
> select MIPS32_COMPAT
>
> Thanks,
> Tiezhu
>