[PATCH v2] riscv: Handle zicsr/zifencei issue between gcc and binutils

[PATCH v2] riscv: Handle zicsr/zifencei issue between gcc and binutils

[Mingzheng Xing]

Binutils-2.38 and GCC-12.1.0 bump[0] default ISA spec to newer version
20191213 which moves some instructions from the I extension to the
Zicsr and Zifencei extensions. So if one of the binutils and GCC exceeds
that version, we should turn on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
to cope with the new changes.

The case of clang is special[1][2], where older clang versions (<17) need
to be rolled back to old ISA spec to fix it. And the less common case,
since older GCC versions (<11.1.0) did not support zicsr and zifencei
extension for -march, also requires a fallback to cope with it.

For more information, please refer to:
commit 6df2a016c0c8 ("riscv: fix build with binutils 2.38")
commit e89c2e815e76 ("riscv: Handle zicsr/zifencei issues between clang and binutils")
Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd [0]
Link: https://lore.kernel.org/all/20230308220842.1231003-1-conor@kernel.org [1]
Link: https://lore.kernel.org/all/20230223220546.52879-1-conor@kernel.org [2]
Link: https://lore.kernel.org/all/20230725170405.251011-1-xingmingzheng@iscas.ac.cn
Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
---

v2:
- Update the Kconfig help text and commit message.
- Add considerations for low version gcc case.

Sorry for the formatting error on my mailing list reply.

 arch/riscv/Kconfig | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 4c07b9189c86..08afd47de157 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -570,24 +570,27 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE
 config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
 	def_bool y
 	# https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
-	depends on AS_IS_GNU && AS_VERSION >= 23800
+	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd
+	depends on GCC_VERSION >= 120100 || (AS_IS_GNU && AS_VERSION >= 23800)
 	help
-	  Newer binutils versions default to ISA spec version 20191213 which
-	  moves some instructions from the I extension to the Zicsr and Zifencei
-	  extensions.
+	  Binutils-2.38 and GCC-12.1.0 bump default ISA spec to newer version
+	  20191213 which moves some instructions from the I extension to the
+	  Zicsr and Zifencei extensions.
 
 config TOOLCHAIN_NEEDS_OLD_ISA_SPEC
 	def_bool y
 	depends on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
 	# https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
-	depends on CC_IS_CLANG && CLANG_VERSION < 170000
+	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b03be74bad08c382da47e048007a78fa3fb4ef49
+	depends on (CC_IS_CLANG && CLANG_VERSION < 170000) || \
+		   (CC_IS_GCC && GCC_VERSION < 110100)
 	help
-	  Certain versions of clang do not support zicsr and zifencei via -march
-	  but newer versions of binutils require it for the reasons noted in the
-	  help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
+	  Certain versions of clang (or GCC) do not support zicsr and zifencei via
+	  -march but newer versions of binutils require it for the reasons noted
+	  in the help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
 	  option causes an older ISA spec compatible with these older versions
-	  of clang to be passed to GAS, which has the same result as passing zicsr
-	  and zifencei to -march.
+	  of clang (or GCC) to be passed to GAS, which has the same result as
+	  passing zicsr and zifencei to -march.
 
 config FPU
 	bool "FPU support"
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv: Handle zicsr/zifencei issue between gcc and binutils

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 4524 bytes --]

On Thu, Jul 27, 2023 at 01:45:24AM +0800, Mingzheng Xing wrote:
> Binutils-2.38 and GCC-12.1.0 bump[0] default ISA spec to newer version
> 20191213 which moves some instructions from the I extension to the
> Zicsr and Zifencei extensions. So if one of the binutils and GCC exceeds
> that version, we should turn on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
> to cope with the new changes.
> 
> The case of clang is special[1][2], where older clang versions (<17) need
> to be rolled back to old ISA spec to fix it. And the less common case,
> since older GCC versions (<11.1.0) did not support zicsr and zifencei

Can you provide a link to explain why this is 11.1.0 in particular?

> extension for -march, also requires a fallback to cope with it.
> 
> For more information, please refer to:
> commit 6df2a016c0c8 ("riscv: fix build with binutils 2.38")
> commit e89c2e815e76 ("riscv: Handle zicsr/zifencei issues between clang and binutils")
> Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd [0]
> Link: https://lore.kernel.org/all/20230308220842.1231003-1-conor@kernel.org [1]
> Link: https://lore.kernel.org/all/20230223220546.52879-1-conor@kernel.org [2]

> Link: https://lore.kernel.org/all/20230725170405.251011-1-xingmingzheng@iscas.ac.cn

This shouldn't be here, you don't link to your old patches.

> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>

This is still broken for:
CONFIG_CLANG_VERSION=0
CONFIG_AS_IS_GNU=y
CONFIG_AS_VERSION=23500
CONFIG_LD_IS_BFD=y
CONFIG_LD_VERSION=23500

Please don't post a v2 while there is still ongoing discussion on the
v1. I'll try to reply here tomorrow with a diff you can fold in to fix
the problem.

Thanks,
Conor.

> ---
> 
> v2:
> - Update the Kconfig help text and commit message.
> - Add considerations for low version gcc case.
> 
> Sorry for the formatting error on my mailing list reply.
> 
>  arch/riscv/Kconfig | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 4c07b9189c86..08afd47de157 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -570,24 +570,27 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE
>  config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
>  	def_bool y
>  	# https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
> -	depends on AS_IS_GNU && AS_VERSION >= 23800
> +	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd
> +	depends on GCC_VERSION >= 120100 || (AS_IS_GNU && AS_VERSION >= 23800)
>  	help
> -	  Newer binutils versions default to ISA spec version 20191213 which
> -	  moves some instructions from the I extension to the Zicsr and Zifencei
> -	  extensions.
> +	  Binutils-2.38 and GCC-12.1.0 bump default ISA spec to newer version
> +	  20191213 which moves some instructions from the I extension to the
> +	  Zicsr and Zifencei extensions.
>  
>  config TOOLCHAIN_NEEDS_OLD_ISA_SPEC
>  	def_bool y
>  	depends on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
>  	# https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
> -	depends on CC_IS_CLANG && CLANG_VERSION < 170000
> +	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b03be74bad08c382da47e048007a78fa3fb4ef49
> +	depends on (CC_IS_CLANG && CLANG_VERSION < 170000) || \
> +		   (CC_IS_GCC && GCC_VERSION < 110100)
>  	help
> -	  Certain versions of clang do not support zicsr and zifencei via -march
> -	  but newer versions of binutils require it for the reasons noted in the
> -	  help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
> +	  Certain versions of clang (or GCC) do not support zicsr and zifencei via
> +	  -march but newer versions of binutils require it for the reasons noted
> +	  in the help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
>  	  option causes an older ISA spec compatible with these older versions
> -	  of clang to be passed to GAS, which has the same result as passing zicsr
> -	  and zifencei to -march.
> +	  of clang (or GCC) to be passed to GAS, which has the same result as
> +	  passing zicsr and zifencei to -march.
>  
>  config FPU
>  	bool "FPU support"
> -- 
> 2.34.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv: Handle zicsr/zifencei issue between gcc and binutils

[Mingzheng Xing]

On 7/27/23 02:02, Conor Dooley wrote:
> On Thu, Jul 27, 2023 at 01:45:24AM +0800, Mingzheng Xing wrote:
>> Binutils-2.38 and GCC-12.1.0 bump[0] default ISA spec to newer version
>> 20191213 which moves some instructions from the I extension to the
>> Zicsr and Zifencei extensions. So if one of the binutils and GCC exceeds
>> that version, we should turn on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
>> to cope with the new changes.
>>
>> The case of clang is special[1][2], where older clang versions (<17) need
>> to be rolled back to old ISA spec to fix it. And the less common case,
>> since older GCC versions (<11.1.0) did not support zicsr and zifencei
> Can you provide a link to explain why this is 11.1.0 in particular?

Okay, I can add it in commit message. gcc-11.1.0 is particular
because it add support zicsr and zifencei extension for -march[1].

Link: 
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b03be74bad08c382da47e048007a78fa3fb4ef49 
[1]

>> extension for -march, also requires a fallback to cope with it.
>>
>> For more information, please refer to:
>> commit 6df2a016c0c8 ("riscv: fix build with binutils 2.38")
>> commit e89c2e815e76 ("riscv: Handle zicsr/zifencei issues between clang and binutils")
>> Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd [0]
>> Link: https://lore.kernel.org/all/20230308220842.1231003-1-conor@kernel.org [1]
>> Link: https://lore.kernel.org/all/20230223220546.52879-1-conor@kernel.org [2]
>> Link: https://lore.kernel.org/all/20230725170405.251011-1-xingmingzheng@iscas.ac.cn
> This shouldn't be here, you don't link to your old patches.

My bad, I will fix it.

>
>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
> This is still broken for:
> CONFIG_CLANG_VERSION=0
> CONFIG_AS_IS_GNU=y
> CONFIG_AS_VERSION=23500
> CONFIG_LD_IS_BFD=y
> CONFIG_LD_VERSION=23500

Do you mean that these CONFIG_* will cause kernel
compilation errors when paired with certain versions of GCC?
Or perhaps I misunderstood your meaning.

>
> Please don't post a v2 while there is still ongoing discussion on the
> v1. I'll try to reply here tomorrow with a diff you can fold in to fix
> the problem.

Okay, thanks for your review.

> Thanks,
> Conor.
>
>> ---
>>
>> v2:
>> - Update the Kconfig help text and commit message.
>> - Add considerations for low version gcc case.
>>
>> Sorry for the formatting error on my mailing list reply.
>>
>>   arch/riscv/Kconfig | 23 +++++++++++++----------
>>   1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 4c07b9189c86..08afd47de157 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -570,24 +570,27 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE
>>   config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
>>   	def_bool y
>>   	# https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
>> -	depends on AS_IS_GNU && AS_VERSION >= 23800
>> +	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd
>> +	depends on GCC_VERSION >= 120100 || (AS_IS_GNU && AS_VERSION >= 23800)
>>   	help
>> -	  Newer binutils versions default to ISA spec version 20191213 which
>> -	  moves some instructions from the I extension to the Zicsr and Zifencei
>> -	  extensions.
>> +	  Binutils-2.38 and GCC-12.1.0 bump default ISA spec to newer version
>> +	  20191213 which moves some instructions from the I extension to the
>> +	  Zicsr and Zifencei extensions.
>>   
>>   config TOOLCHAIN_NEEDS_OLD_ISA_SPEC
>>   	def_bool y
>>   	depends on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
>>   	# https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
>> -	depends on CC_IS_CLANG && CLANG_VERSION < 170000
>> +	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b03be74bad08c382da47e048007a78fa3fb4ef49
>> +	depends on (CC_IS_CLANG && CLANG_VERSION < 170000) || \
>> +		   (CC_IS_GCC && GCC_VERSION < 110100)
>>   	help
>> -	  Certain versions of clang do not support zicsr and zifencei via -march
>> -	  but newer versions of binutils require it for the reasons noted in the
>> -	  help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
>> +	  Certain versions of clang (or GCC) do not support zicsr and zifencei via
>> +	  -march but newer versions of binutils require it for the reasons noted
>> +	  in the help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
>>   	  option causes an older ISA spec compatible with these older versions
>> -	  of clang to be passed to GAS, which has the same result as passing zicsr
>> -	  and zifencei to -march.
>> +	  of clang (or GCC) to be passed to GAS, which has the same result as
>> +	  passing zicsr and zifencei to -march.
>>   
>>   config FPU
>>   	bool "FPU support"
>> -- 
>> 2.34.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv: Handle zicsr/zifencei issue between gcc and binutils

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1171 bytes --]

On Thu, Jul 27, 2023 at 03:34:16AM +0800, Mingzheng Xing wrote:
> On 7/27/23 02:02, Conor Dooley wrote:

> > This is still broken for:
> > CONFIG_CLANG_VERSION=0
> > CONFIG_AS_IS_GNU=y
> > CONFIG_AS_VERSION=23500
> > CONFIG_LD_IS_BFD=y
> > CONFIG_LD_VERSION=23500
> 
> Do you mean that these CONFIG_* will cause kernel
> compilation errors when paired with certain versions of GCC?
> Or perhaps I misunderstood your meaning.

No, this section is generated by kconfig, although I messed up my
trimming of the list & accidentally removed the gcc version, rather
than the clang version. Here's the full thing:

CONFIG_CC_VERSION_TEXT="riscv64-unknown-linux-gnu-gcc (g2ee5e430018) 12.2.0"
CONFIG_CC_IS_GCC=y
CONFIG_GCC_VERSION=120200
CONFIG_CLANG_VERSION=0
CONFIG_AS_IS_GNU=y
CONFIG_AS_VERSION=23500
CONFIG_LD_IS_BFD=y
CONFIG_LD_VERSION=23500
CONFIG_LLD_VERSION=0
CONFIG_CC_CAN_LINK=y
CONFIG_CC_CAN_LINK_STATIC=y
CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT=y
CONFIG_CC_HAS_ASM_INLINE=y
CONFIG_CC_HAS_NO_PROFILE_FN_ATTR=y
CONFIG_PAHOLE_VERSION=0
CONFIG_CONSTRUCTORS=y
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_TABLE_SORT=y

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv: Handle zicsr/zifencei issue between gcc and binutils

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3664 bytes --]

On Wed, Jul 26, 2023 at 08:41:55PM +0100, Conor Dooley wrote:
> On Thu, Jul 27, 2023 at 03:34:16AM +0800, Mingzheng Xing wrote:
> > On 7/27/23 02:02, Conor Dooley wrote:
> 
> > > This is still broken for:
> > > CONFIG_CLANG_VERSION=0
> > > CONFIG_AS_IS_GNU=y
> > > CONFIG_AS_VERSION=23500
> > > CONFIG_LD_IS_BFD=y
> > > CONFIG_LD_VERSION=23500
> > 
> > Do you mean that these CONFIG_* will cause kernel
> > compilation errors when paired with certain versions of GCC?
> > Or perhaps I misunderstood your meaning.
> 
> No, this section is generated by kconfig, although I messed up my
> trimming of the list & accidentally removed the gcc version, rather
> than the clang version. Here's the full thing:
> 
> CONFIG_CC_VERSION_TEXT="riscv64-unknown-linux-gnu-gcc (g2ee5e430018) 12.2.0"
> CONFIG_CC_IS_GCC=y
> CONFIG_GCC_VERSION=120200
> CONFIG_CLANG_VERSION=0
> CONFIG_AS_IS_GNU=y
> CONFIG_AS_VERSION=23500
> CONFIG_LD_IS_BFD=y
> CONFIG_LD_VERSION=23500
> CONFIG_LLD_VERSION=0
> CONFIG_CC_CAN_LINK=y
> CONFIG_CC_CAN_LINK_STATIC=y
> CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
> CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT=y
> CONFIG_CC_HAS_ASM_INLINE=y
> CONFIG_CC_HAS_NO_PROFILE_FN_ATTR=y
> CONFIG_PAHOLE_VERSION=0
> CONFIG_CONSTRUCTORS=y
> CONFIG_IRQ_WORK=y
> CONFIG_BUILDTIME_TABLE_SORT=y

I think this should sort things out for the even-older binutils case. I
took the opportunity to fix some grammatical issues that seem to have
snuck into the help text in your patch & to drop the \, since the
depends on fits in one line.

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index e1b66ee88323..2d0d89213c97 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -571,25 +571,27 @@ config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
 	def_bool y
 	# https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
 	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd
-	depends on GCC_VERSION >= 120100 || (AS_IS_GNU && AS_VERSION >= 23800)
+	depends on AS_IS_GNU
+	depends on (GCC_VERSION >= 120100 && AS_VERSION >= 23600) || AS_VERSION >= 23800
 	help
-	  Binutils-2.38 and GCC-12.1.0 bump default ISA spec to newer version
+	  Binutils-2.38 and GCC-12.1.0 bump the default ISA spec to version
 	  20191213 which moves some instructions from the I extension to the
-	  Zicsr and Zifencei extensions.
+	  Zicsr and Zifencei extensions. On the other hand, Binutils prior to
+	  2.35 does not understand these arguments and will error if they are
+	  passed.
 
 config TOOLCHAIN_NEEDS_OLD_ISA_SPEC
 	def_bool y
 	depends on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
 	# https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
 	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b03be74bad08c382da47e048007a78fa3fb4ef49
-	depends on (CC_IS_CLANG && CLANG_VERSION < 170000) || \
-		   (CC_IS_GCC && GCC_VERSION < 110100)
+	depends on (CC_IS_CLANG && CLANG_VERSION < 170000) || (CC_IS_GCC && GCC_VERSION < 110100)
 	help
-	  Certain versions of clang (or GCC) do not support zicsr and zifencei via
+	  Certain versions of clang and GCC do not support zicsr and zifencei via
 	  -march but newer versions of binutils require it for the reasons noted
 	  in the help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
 	  option causes an older ISA spec compatible with these older versions
-	  of clang (or GCC) to be passed to GAS, which has the same result as
+	  of clang and GCC to be passed to GAS, which has the same result as
 	  passing zicsr and zifencei to -march.
 
 config FPU


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv: Handle zicsr/zifencei issue between gcc and binutils

[Mingzheng Xing]

On 7/27/23 15:53, Conor Dooley wrote:
> On Wed, Jul 26, 2023 at 08:41:55PM +0100, Conor Dooley wrote:
>> On Thu, Jul 27, 2023 at 03:34:16AM +0800, Mingzheng Xing wrote:
>>> On 7/27/23 02:02, Conor Dooley wrote:
>>>> This is still broken for:
>>>> CONFIG_CLANG_VERSION=0
>>>> CONFIG_AS_IS_GNU=y
>>>> CONFIG_AS_VERSION=23500
>>>> CONFIG_LD_IS_BFD=y
>>>> CONFIG_LD_VERSION=23500
>>> Do you mean that these CONFIG_* will cause kernel
>>> compilation errors when paired with certain versions of GCC?
>>> Or perhaps I misunderstood your meaning.
>> No, this section is generated by kconfig, although I messed up my
>> trimming of the list & accidentally removed the gcc version, rather
>> than the clang version. Here's the full thing:
>>
>> CONFIG_CC_VERSION_TEXT="riscv64-unknown-linux-gnu-gcc (g2ee5e430018) 12.2.0"
>> CONFIG_CC_IS_GCC=y
>> CONFIG_GCC_VERSION=120200
>> CONFIG_CLANG_VERSION=0
>> CONFIG_AS_IS_GNU=y
>> CONFIG_AS_VERSION=23500
>> CONFIG_LD_IS_BFD=y
>> CONFIG_LD_VERSION=23500
>> CONFIG_LLD_VERSION=0
>> CONFIG_CC_CAN_LINK=y
>> CONFIG_CC_CAN_LINK_STATIC=y
>> CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
>> CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT=y
>> CONFIG_CC_HAS_ASM_INLINE=y
>> CONFIG_CC_HAS_NO_PROFILE_FN_ATTR=y
>> CONFIG_PAHOLE_VERSION=0
>> CONFIG_CONSTRUCTORS=y
>> CONFIG_IRQ_WORK=y
>> CONFIG_BUILDTIME_TABLE_SORT=y
> I think this should sort things out for the even-older binutils case. I
> took the opportunity to fix some grammatical issues that seem to have
> snuck into the help text in your patch & to drop the \, since the
> depends on fits in one line.

hi, Conor.

I reproduced the error with gcc-12.2.0 and binutils-2.35. I tried a
different solution, which I think makes the logic easier. Showing
the new patch code:

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 4c07b9189c86..a6fa1eed895c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -569,25 +569,24 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE

  config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
         def_bool y
-       # https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
-       depends on AS_IS_GNU && AS_VERSION >= 23800
+       depends on AS_IS_GNU && AS_VERSION >= 23600
         help
-         Newer binutils versions default to ISA spec version 20191213 which
-         moves some instructions from the I extension to the Zicsr and Zifencei
-         extensions.
+         Binutils has supported zicsr and zifencei extensions since version 2.36,
+         try to adapt to the changes by using explicit zicsr and zifencei via
+         -march. For two special cases, where clang<17 or gcc<11.1.0, we will
+         deal with them in CONFIG_TOOLCHAIN_NEEDS_OLD_ISA_SPEC.

  config TOOLCHAIN_NEEDS_OLD_ISA_SPEC
         def_bool y
         depends on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
         # https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
-       depends on CC_IS_CLANG && CLANG_VERSION < 170000
-       help
-         Certain versions of clang do not support zicsr and zifencei via -march
-         but newer versions of binutils require it for the reasons noted in the
-         help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
-         option causes an older ISA spec compatible with these older versions
-         of clang to be passed to GAS, which has the same result as passing zicsr
-         and zifencei to -march.
+       # https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b03be74bad08c382da47e048007a78fa3fb4ef49
+       depends on (CC_IS_CLANG && CLANG_VERSION < 170000) || (CC_IS_GCC && GCC_VERSION < 110100)
+       help
+         Certain versions of clang and GCC do not support zicsr and zifencei via
+         -march. This option causes an older ISA spec compatible with these older
+         versions of clang and GCC to be passed to GAS, which has the same result
+         as passing zicsr and zifencei to -march.

  config FPU
         bool "FPU support"
diff --git a/arch/riscv/kernel/compat_vdso/Makefile b/arch/riscv/kernel/compat_vdso/Makefile
index 189345773e7e..b86e5e2c3aea 100644
--- a/arch/riscv/kernel/compat_vdso/Makefile
+++ b/arch/riscv/kernel/compat_vdso/Makefile
@@ -11,7 +11,13 @@ compat_vdso-syms += flush_icache
  COMPAT_CC := $(CC)
  COMPAT_LD := $(LD)

-COMPAT_CC_FLAGS := -march=rv32g -mabi=ilp32
+# binutils 2.35 does not support the zifencei extension, but in the ISA
+# spec 20191213, G stands for IMAFD_ZICSR_ZIFENCEI.
+ifdef CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
+       COMPAT_CC_FLAGS := -march=rv32g -mabi=ilp32
+else
+       COMPAT_CC_FLAGS := -march=rv32imafd -mabi=ilp32
+endif
  COMPAT_LD_FLAGS := -melf32lriscv

  # Disable attributes, as they're useless and break the build.
-- 
2.34.1


Here are the results of my tests:

gcc          binutils       patched         no patch(test on master)
11.4.0     2.35            ok                  ok
11.4.0     2.36            ok                  ok
11.4.0     2.38            ok                  ok

12.2.0     2.35            ok                  error[1]
12.2.0     2.36            ok                  error[2]
12.2.0     2.38            ok                  ok

10.5.0     2.35            ok                  ok
10.5.0     2.36            ok                  ok
10.5.0     2.38            ok                  error[3]

11.1.0     2.35            ok                  ok
11.1.0     2.36            ok                  ok
11.1.0     2.38            ok                  ok

11.2.0     2.35            ok                  ok
11.2.0     2.36            ok                  ok
11.2.0     2.38            ok                  ok

[1]
Assembler messages:
Fatal error: -march=rv32imafd_zicsr_zifencei: Invalid or unknown z ISA extension: 'zifencei'
make[2]: *** [arch/riscv/kernel/compat_vdso/Makefile:47: arch/riscv/kernel/compat_vdso/rt_sigreturn.o] Error 1

[2]
./arch/riscv/include/asm/vdso/gettimeofday.h: Assembler messages:
./arch/riscv/include/asm/vdso/gettimeofday.h:79: Error: unrecognized opcode `csrr a5,0xc01'
./arch/riscv/include/asm/vdso/gettimeofday.h:79: Error: unrecognized opcode `csrr a5,0xc01'
./arch/riscv/include/asm/vdso/gettimeofday.h:79: Error: unrecognized opcode `csrr a5,0xc01'
./arch/riscv/include/asm/vdso/gettimeofday.h:79: Error: unrecognized opcode `csrr a5,0xc01'
make[2]: *** [scripts/Makefile.build:243: arch/riscv/kernel/vdso/vgettimeofday.o] Error 1

[3]
cc1: error: '-march=rv64imac_zicsr_zifencei': unsupported ISA subset 'z'
cc1: error: ABI requires '-march=rv64'
make[2]: *** [scripts/Makefile.build:243: scripts/mod/empty.o] Error 1
make[2]: *** Waiting for unfinished jobs....
cc1: error: '-march=rv64imac_zicsr_zifencei': unsupported ISA subset 'z'
cc1: error: ABI requires '-march=rv64'

> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index e1b66ee88323..2d0d89213c97 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -571,25 +571,27 @@ config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
>   	def_bool y
>   	# https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
>   	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd
> -	depends on GCC_VERSION >= 120100 || (AS_IS_GNU && AS_VERSION >= 23800)
> +	depends on AS_IS_GNU
> +	depends on (GCC_VERSION >= 120100 && AS_VERSION >= 23600) || AS_VERSION >= 23800

Tests verified that explicit _ZICSR_ZIFENCEI via -march is required
for gcc>=12.1.0, but this only happens for binutils>=2.36,
binutils 2.35 + gcc>=12.1.0 does not need that. Considering
binutils 2.35 together complicates things. So what do you think
of the above new version patch?

Some more info:
- The commit[4] for patch changes.
- binutils 2.36 supports the zifencei extension[5] and splits
zifencei and zicsr from I[6].

[4] commit 0715372a06ce ("riscv: compat: vdso: Add COMPAT_VDSO base code implementation")
[5] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=5a1b31e1e1cee6e9f1c92abff59cdcfff0dddf30
[6] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=729a53530e86972d1143553a415db34e6e01d5d2

Thanks,
Mingzheng.

>   	help
> -	  Binutils-2.38 and GCC-12.1.0 bump default ISA spec to newer version
> +	  Binutils-2.38 and GCC-12.1.0 bump the default ISA spec to version
>   	  20191213 which moves some instructions from the I extension to the
> -	  Zicsr and Zifencei extensions.
> +	  Zicsr and Zifencei extensions. On the other hand, Binutils prior to
> +	  2.35 does not understand these arguments and will error if they are
> +	  passed.
>   
>   config TOOLCHAIN_NEEDS_OLD_ISA_SPEC
>   	def_bool y
>   	depends on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
>   	# https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
>   	# https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b03be74bad08c382da47e048007a78fa3fb4ef49
> -	depends on (CC_IS_CLANG && CLANG_VERSION < 170000) || \
> -		   (CC_IS_GCC && GCC_VERSION < 110100)
> +	depends on (CC_IS_CLANG && CLANG_VERSION < 170000) || (CC_IS_GCC && GCC_VERSION < 110100)
>   	help
> -	  Certain versions of clang (or GCC) do not support zicsr and zifencei via
> +	  Certain versions of clang and GCC do not support zicsr and zifencei via
>   	  -march but newer versions of binutils require it for the reasons noted
>   	  in the help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
>   	  option causes an older ISA spec compatible with these older versions
> -	  of clang (or GCC) to be passed to GAS, which has the same result as
> +	  of clang and GCC to be passed to GAS, which has the same result as
>   	  passing zicsr and zifencei to -march.
>   
>   config FPU
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv: Handle zicsr/zifencei issue between gcc and binutils

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1542 bytes --]

On Sun, Jul 30, 2023 at 01:36:49AM +0800, Mingzheng Xing wrote:

> I reproduced the error with gcc-12.2.0 and binutils-2.35. I tried a
> different solution, which I think makes the logic easier. Showing
> the new patch code:

It is indeed simpler, neat.

> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 4c07b9189c86..a6fa1eed895c 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -569,25 +569,24 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE
> 
>  config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
>         def_bool y
> -       # https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
> -       depends on AS_IS_GNU && AS_VERSION >= 23800
> +       depends on AS_IS_GNU && AS_VERSION >= 23600
>         help
> -         Newer binutils versions default to ISA spec version 20191213 which
> -         moves some instructions from the I extension to the Zicsr and Zifencei
> -         extensions.
> +         Binutils has supported zicsr and zifencei extensions since version 2.36,
> +         try to adapt to the changes by using explicit zicsr and zifencei via
> +         -march.

This sentence no longer makes sense to me, the motivation for why we are
doing this is lost. Please preserve the link & explanation about the
20191213 version of the spec, adding to it the commentary about how we
can relax the check to 2.36, since that makes our lives easier.

The rest of this looks fine to me, if you resubmit I'll look at it
further on Monday.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv: Handle zicsr/zifencei issue between gcc and binutils

[Mingzheng Xing]

On 7/30/23 01:48, Conor Dooley wrote:
> On Sun, Jul 30, 2023 at 01:36:49AM +0800, Mingzheng Xing wrote:
>
>> I reproduced the error with gcc-12.2.0 and binutils-2.35. I tried a
>> different solution, which I think makes the logic easier. Showing
>> the new patch code:
> It is indeed simpler, neat.
>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 4c07b9189c86..a6fa1eed895c 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -569,25 +569,24 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE
>>
>>   config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
>>          def_bool y
>> -       # https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
>> -       depends on AS_IS_GNU && AS_VERSION >= 23800
>> +       depends on AS_IS_GNU && AS_VERSION >= 23600
>>          help
>> -         Newer binutils versions default to ISA spec version 20191213 which
>> -         moves some instructions from the I extension to the Zicsr and Zifencei
>> -         extensions.
>> +         Binutils has supported zicsr and zifencei extensions since version 2.36,
>> +         try to adapt to the changes by using explicit zicsr and zifencei via
>> +         -march.
> This sentence no longer makes sense to me, the motivation for why we are
> doing this is lost. Please preserve the link & explanation about the
> 20191213 version of the spec, adding to it the commentary about how we
> can relax the check to 2.36, since that makes our lives easier.
>
> The rest of this looks fine to me, if you resubmit I'll look at it
> further on Monday.

I updated it in v3 [1]. Thanks for your review.

[1] https://lore.kernel.org/all/20230731095936.23397-1-xingmingzheng@iscas.ac.cn

> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv