diff mbox series

MIPS: Add comment about CONFIG_MIPS32_O32 in loongson3_defconfig when build with Clang

Message ID 1614820544-10686-1-git-send-email-yangtiezhu@loongson.cn (mailing list archive)
State Superseded
Headers show
Series MIPS: Add comment about CONFIG_MIPS32_O32 in loongson3_defconfig when build with Clang | expand

Commit Message

Tiezhu Yang March 4, 2021, 1:15 a.m. UTC
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 <yangtiezhu@loongson.cn>
---
 arch/mips/configs/loongson3_defconfig | 3 +++
 1 file changed, 3 insertions(+)

Comments

Nathan Chancellor March 4, 2021, 2:02 a.m. UTC | #1
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 <yangtiezhu@loongson.cn>
> ---
>  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
Tiezhu Yang March 4, 2021, 3:48 a.m. UTC | #2
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
Nathan Chancellor March 4, 2021, 5:18 a.m. UTC | #3
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
>
Maciej W. Rozycki March 4, 2021, 11:08 p.m. UTC | #4
On Thu, 4 Mar 2021, Tiezhu Yang wrote:

> 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".

 Huh?  Is that a joke?  From the o32 psABI's point of view a MIPS64 CPU is 
exactly the same as a MIPS32 one (for whatever ISA revision), so there's 
nothing to support there really other than the CPU/ISA name.

 As much as I dislike all the hacks the Clang community seems to come up 
with for the shortcomings of their tool there has to be a saner workaround 
available rather than forcibly disabling support for the o32 ABI with 
CONFIG_64BIT kernels, but the report is missing the compiler invocation 
line triggering the issue (V=1 perhaps?), which should be included with 
any commit description anyway, so I can't suggest anything based on the 
limited information provided.

  Maciej
Jiaxun Yang March 5, 2021, 2:22 a.m. UTC | #5
在 2021/3/5 上午7:08, Maciej W. Rozycki 写道:
> On Thu, 4 Mar 2021, Tiezhu Yang wrote:
> 
>> 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".
> 
>   Huh?  Is that a joke?  From the o32 psABI's point of view a MIPS64 CPU is
> exactly the same as a MIPS32 one (for whatever ISA revision), so there's
> nothing to support there really other than the CPU/ISA name.

Clang treat MIPS32 as a different backend so we may need some extra 
effort....

TBH it is a toolchain issue and kernel workaround seems bogus.

 From my point view we can "s/mips64r2/mips32r2" when doing syscall checks
to workaround clang issue instead of disable it for kernel.

Thanks.

- Jiaxun

> 
>   As much as I dislike all the hacks the Clang community seems to come up
> with for the shortcomings of their tool there has to be a saner workaround
> available rather than forcibly disabling support for the o32 ABI with
> CONFIG_64BIT kernels, but the report is missing the compiler invocation
> line triggering the issue (V=1 perhaps?), which should be included with
> any commit description anyway, so I can't suggest anything based on the
> limited information provided.
> 
>    Maciej
>
Maciej W. Rozycki March 5, 2021, 2:46 a.m. UTC | #6
On Fri, 5 Mar 2021, Jiaxun Yang wrote:

> >   Huh?  Is that a joke?  From the o32 psABI's point of view a MIPS64 CPU is
> > exactly the same as a MIPS32 one (for whatever ISA revision), so there's
> > nothing to support there really other than the CPU/ISA name.
> 
> Clang treat MIPS32 as a different backend so we may need some extra effort....
> 
> TBH it is a toolchain issue and kernel workaround seems bogus.
> 
> From my point view we can "s/mips64r2/mips32r2" when doing syscall checks
> to workaround clang issue instead of disable it for kernel.

 I had something like this in mind, but obviously that has to be done in a 
consistent manner across all the possible 64-bit `-march=...' selections, 
as I suppose that is where the problem comes from.  So we'd have to handle 
things like say `octeon'.  But I'd like to see the invocation line to be 
sure (I could try and check that myself, but I don't have Clang and it's 
the patch submitter's job anyway to explain things and not the reviewer's 
to chase them).

 Maybe we could cheat and wire everything to a single setting so as to 
keep the hack to the minimum, but we need to know what the right setting 
is from the Clang people.

 NB only MIPS IV is special in that it has 32-bit extensions beyond MIPS 
III but not a corresponding 32-bit ISA.  The corresponding 32-bit ISA for 
MIPS III is MIPS II, and obviously all the modern MIPS ISAs come in pairs.  
So it's only MIPS IV where o32 may want special support (at the hardware 
level the ISA had the CP0.Status.XX bit to control the ISA extensions), 
and I guess only a few people care at this point, though some are present 
on this mailing list.

  Maciej
diff mbox series

Patch

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