| Message ID | 20230323145924.4194-20-andy.chiu@sifive.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: Add vector ISA support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/apply | fail | Patch does not apply to for-next |
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
On Thu, Mar 23, 2023 at 02:59:23PM +0000, Andy Chiu wrote: > Some extensions use .option arch directive to selectively enable certain > extensions in parts of its assembly code. For example, Zbb uses it to > inform assmebler to emit bit manipulation instructions. However, > supporting of this directive only exist on GNU assembler and has not > landed on clang at the moment, making TOOLCHAIN_HAS_ZBB depend on > AS_IS_GNU. > > While it is still under review at https://reviews.llvm.org/D123515, the > upcoming Vector patch also requires this feature in assembler. Thus, > provide Kconfig AS_HAS_OPTION_ARCH to detect such feature. Then > TOOLCHAIN_HAS_XXX will be turned on automatically when the feature land. > > Suggested-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > --- > arch/riscv/Kconfig | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 36a5b6fed0d3..4f8fd4002f1d 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -244,6 +244,12 @@ config RISCV_DMA_NONCOHERENT > config AS_HAS_INSN > def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero) > > +config AS_HAS_OPTION_ARCH > + # https://reviews.llvm.org/D123515 > + def_bool y > + depends on $(as-instr, .option arch$(comma) +m) > + depends on !$(as-instr, .option arch$(comma) -i) Oh cool, I didn't expect this to work given what Nathan said in his mail, but I gave it a whirl and it does seem to. I suppose: Reviewed-by: Conor Dooley <conor.dooley@microchip.com> I'd rather it be this way so that it is "hands off", as opposed to the version check that would need updating in the future. And I guess it means that support for V & IAS will automatically turn on for stable kernels too once the LLVM change lands, which is nice ;) Thanks Andy! > + > source "arch/riscv/Kconfig.socs" > source "arch/riscv/Kconfig.errata" > > @@ -442,7 +448,7 @@ config TOOLCHAIN_HAS_ZBB > depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zbb) > depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zbb) > depends on LLD_VERSION >= 150000 || LD_VERSION >= 23900 > - depends on AS_IS_GNU > + depends on AS_HAS_OPTION_ARCH > > config RISCV_ISA_ZBB > bool "Zbb extension support for bit manipulation instructions" > -- > 2.17.1 > >
On Thu, Mar 23, 2023 at 03:26:14PM +0000, Conor Dooley wrote: > On Thu, Mar 23, 2023 at 02:59:23PM +0000, Andy Chiu wrote: > > Some extensions use .option arch directive to selectively enable certain > > extensions in parts of its assembly code. For example, Zbb uses it to > > inform assmebler to emit bit manipulation instructions. However, > > supporting of this directive only exist on GNU assembler and has not > > landed on clang at the moment, making TOOLCHAIN_HAS_ZBB depend on > > AS_IS_GNU. > > > > While it is still under review at https://reviews.llvm.org/D123515, the > > upcoming Vector patch also requires this feature in assembler. Thus, > > provide Kconfig AS_HAS_OPTION_ARCH to detect such feature. Then > > TOOLCHAIN_HAS_XXX will be turned on automatically when the feature land. > > > > Suggested-by: Nathan Chancellor <nathan@kernel.org> > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > --- > > arch/riscv/Kconfig | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index 36a5b6fed0d3..4f8fd4002f1d 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -244,6 +244,12 @@ config RISCV_DMA_NONCOHERENT > > config AS_HAS_INSN > > def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero) > > > > +config AS_HAS_OPTION_ARCH > > + # https://reviews.llvm.org/D123515 > > + def_bool y > > + depends on $(as-instr, .option arch$(comma) +m) > > + depends on !$(as-instr, .option arch$(comma) -i) > > Oh cool, I didn't expect this to work given what Nathan said in his > mail, but I gave it a whirl and it does seem to. The second line is the clever part of this option that I had not considered, as it checks for something that should error in addition to something that shouldn't:: $ echo '.option arch, -i' | riscv64-linux-gcc -c -o /dev/null -x assembler - {standard input}: Assembler messages: {standard input}:1: Error: cannot + or - base extension `i' in .option arch `-i' Looking at D123515, I see this same option test is present and appears to error in the same manner so this should work when that change is merged. Reviewed-by: Nathan Chancellor <nathan@kernel.org> > I suppose: > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > I'd rather it be this way so that it is "hands off", as opposed to the > version check that would need updating in the future. And I guess it > means that support for V & IAS will automatically turn on for stable > kernels too once the LLVM change lands, which is nice ;) Very much agreed! > Thanks Andy! > > > + > > source "arch/riscv/Kconfig.socs" > > source "arch/riscv/Kconfig.errata" > > > > @@ -442,7 +448,7 @@ config TOOLCHAIN_HAS_ZBB > > depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zbb) > > depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zbb) > > depends on LLD_VERSION >= 150000 || LD_VERSION >= 23900 > > - depends on AS_IS_GNU > > + depends on AS_HAS_OPTION_ARCH > > > > config RISCV_ISA_ZBB > > bool "Zbb extension support for bit manipulation instructions" > > -- > > 2.17.1 > > > >
On Fri, Mar 24, 2023 at 10:59 PM Nathan Chancellor <nathan@kernel.org> wrote: > > On Thu, Mar 23, 2023 at 03:26:14PM +0000, Conor Dooley wrote: > > On Thu, Mar 23, 2023 at 02:59:23PM +0000, Andy Chiu wrote: > > > Some extensions use .option arch directive to selectively enable certain > > > extensions in parts of its assembly code. For example, Zbb uses it to > > > inform assmebler to emit bit manipulation instructions. However, > > > supporting of this directive only exist on GNU assembler and has not > > > landed on clang at the moment, making TOOLCHAIN_HAS_ZBB depend on > > > AS_IS_GNU. > > > > > > While it is still under review at https://reviews.llvm.org/D123515, the > > > upcoming Vector patch also requires this feature in assembler. Thus, > > > provide Kconfig AS_HAS_OPTION_ARCH to detect such feature. Then > > > TOOLCHAIN_HAS_XXX will be turned on automatically when the feature land. > > > > > > Suggested-by: Nathan Chancellor <nathan@kernel.org> > > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > > --- > > > arch/riscv/Kconfig | 8 +++++++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > index 36a5b6fed0d3..4f8fd4002f1d 100644 > > > --- a/arch/riscv/Kconfig > > > +++ b/arch/riscv/Kconfig > > > @@ -244,6 +244,12 @@ config RISCV_DMA_NONCOHERENT > > > config AS_HAS_INSN > > > def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero) > > > > > > +config AS_HAS_OPTION_ARCH > > > + # https://reviews.llvm.org/D123515 > > > + def_bool y > > > + depends on $(as-instr, .option arch$(comma) +m) > > > + depends on !$(as-instr, .option arch$(comma) -i) > > > > Oh cool, I didn't expect this to work given what Nathan said in his > > mail, but I gave it a whirl and it does seem to. > > The second line is the clever part of this option that I had not > considered, as it checks for something that should error in addition to > something that shouldn't:: > > $ echo '.option arch, -i' | riscv64-linux-gcc -c -o /dev/null -x assembler - > {standard input}: Assembler messages: > {standard input}:1: Error: cannot + or - base extension `i' in .option arch `-i' > > Looking at D123515, I see this same option test is present and appears > to error in the same manner so this should work when that change is > merged. Thanks for checking D123515 Nathan. It was just lucky that It gets the same test coverage in Clang. I would take a look at that aspect in the future if the same happens. > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > > I suppose: > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > > > I'd rather it be this way so that it is "hands off", as opposed to the > > version check that would need updating in the future. And I guess it > > means that support for V & IAS will automatically turn on for stable > > kernels too once the LLVM change lands, which is nice ;) > > Very much agreed! > > > Thanks Andy! > > > > > + > > > source "arch/riscv/Kconfig.socs" > > > source "arch/riscv/Kconfig.errata" > > > > > > @@ -442,7 +448,7 @@ config TOOLCHAIN_HAS_ZBB > > > depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zbb) > > > depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zbb) > > > depends on LLD_VERSION >= 150000 || LD_VERSION >= 23900 > > > - depends on AS_IS_GNU > > > + depends on AS_HAS_OPTION_ARCH > > > > > > config RISCV_ISA_ZBB > > > bool "Zbb extension support for bit manipulation instructions" > > > -- > > > 2.17.1 > > > > > > > > Regards, Andy
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 36a5b6fed0d3..4f8fd4002f1d 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -244,6 +244,12 @@ config RISCV_DMA_NONCOHERENT config AS_HAS_INSN def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero) +config AS_HAS_OPTION_ARCH + # https://reviews.llvm.org/D123515 + def_bool y + depends on $(as-instr, .option arch$(comma) +m) + depends on !$(as-instr, .option arch$(comma) -i) + source "arch/riscv/Kconfig.socs" source "arch/riscv/Kconfig.errata" @@ -442,7 +448,7 @@ config TOOLCHAIN_HAS_ZBB depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zbb) depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zbb) depends on LLD_VERSION >= 150000 || LD_VERSION >= 23900 - depends on AS_IS_GNU + depends on AS_HAS_OPTION_ARCH config RISCV_ISA_ZBB bool "Zbb extension support for bit manipulation instructions"
Some extensions use .option arch directive to selectively enable certain extensions in parts of its assembly code. For example, Zbb uses it to inform assmebler to emit bit manipulation instructions. However, supporting of this directive only exist on GNU assembler and has not landed on clang at the moment, making TOOLCHAIN_HAS_ZBB depend on AS_IS_GNU. While it is still under review at https://reviews.llvm.org/D123515, the upcoming Vector patch also requires this feature in assembler. Thus, provide Kconfig AS_HAS_OPTION_ARCH to detect such feature. Then TOOLCHAIN_HAS_XXX will be turned on automatically when the feature land. Suggested-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Andy Chiu <andy.chiu@sifive.com> --- arch/riscv/Kconfig | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)