| Message ID | 20230726174524.340952-1-xingmingzheng@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] riscv: Handle zicsr/zifencei issue between gcc and binutils | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 471aba2e4760 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 37 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
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
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
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
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
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.
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.
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
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"
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(-)