| Message ID | 20221216185012.2342675-1-abdulras@google.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | riscv: avoid enabling vectorized code generation | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/patch_count | success | Link |
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 57 and now 57 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/build_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 10 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| 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 |
Hey Saleem, On 16 December 2022 10:50:12 GMT-08:00, Saleem Abdulrasool <abdulras@google.com> wrote: >The compiler is free to generate vectorized operations for zero'ing >memory. The kernel does not use the vector unit on RISCV, similar to >architectures such as x86 where we use `-mno-mmx` et al to prevent the >implicit vectorization. Perform a similar check for >`-mno-implicit-float` to avoid this on RISC-V targets. > >Signed-off-by: Saleem Abdulrasool <abdulras@google.com> >--- > arch/riscv/Makefile | 4 ++++ > 1 file changed, 4 insertions(+) > >diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile >index 0d13b597cb55..68433476a96e 100644 >--- a/arch/riscv/Makefile >+++ b/arch/riscv/Makefile >@@ -89,6 +89,10 @@ KBUILD_AFLAGS_MODULE += $(call as-option,-Wa$(comma)-mno-relax) > # architectures. It's faster to have GCC emit only aligned accesses. > KBUILD_CFLAGS += $(call cc-option,-mstrict-align) > >+# Ensure that we do not vectorize the kernel code when the `v` extension is >+# enabled. This mirrors the `-mno-mmx` et al on x86. The v extension should not be enabled at all though, right? Excuse my naivity here, but what am I missing? The vector support thread is here: https://lore.kernel.org/linux-riscv/20220921214439.1491510-1-stillson@rivosinc.com/ What have I missed that causes a problem without that patchset? And if I have missed something, this patch needs to be cc: stable? Thanks, Conor. >+KBUILD_CFLAGS += $(call cc-option,-mno-implicit-float) >+ > ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y) > prepare: stack_protector_prepare > stack_protector_prepare: prepare0
On 2022-12-16 18:50, Saleem Abdulrasool wrote: > The compiler is free to generate vectorized operations for zero'ing > memory. The kernel does not use the vector unit on RISCV, similar to > architectures such as x86 where we use `-mno-mmx` et al to prevent the > implicit vectorization. Perform a similar check for > `-mno-implicit-float` to avoid this on RISC-V targets. I'm not sure if we should be emitting either of the vector or floating point instrucitons in the kernel without explicitly marking the section of code which is using them such as specific accelerator blocks.
On Fri, 16 Dec 2022 11:45:21 PST (-0800), ben.dooks@codethink.co.uk wrote: > > > On 2022-12-16 18:50, Saleem Abdulrasool wrote: >> The compiler is free to generate vectorized operations for zero'ing >> memory. The kernel does not use the vector unit on RISCV, similar to >> architectures such as x86 where we use `-mno-mmx` et al to prevent the >> implicit vectorization. Perform a similar check for >> `-mno-implicit-float` to avoid this on RISC-V targets. > > I'm not sure if we should be emitting either of the vector or floating > point instrucitons in the kernel without explicitly marking the section > of code which is using them such as specific accelerator blocks. Yep, we can't let the compiler just blindly enable V or F/D. V would very much break things as we have no support, but even when that's in we'll we at roughly the same spot as F/D are now where we need to handle the lazy save/restore bits. This looks like an LLVM-only option, I see at least some handling here https://github.com/llvm/llvm-project/blob/a72883b7612f5c00b592da85ed2f1fd81258cc08/clang/lib/Driver/ToolChains/Clang.cpp#L2098 but I don't really know LLVM enough to understand if there's some default for `-mimplicit-float` and I can't find anything in the docs. If it can be turned on by default and that results in F/D/V instructions then we'll need to explicitly turn it off, and that would need to be backported. Maybe Nick or Nathan knows what's up here?
On Fri, Dec 16, 2022 at 11:54 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Fri, 16 Dec 2022 11:45:21 PST (-0800), ben.dooks@codethink.co.uk wrote: > > > > > > On 2022-12-16 18:50, Saleem Abdulrasool wrote: > >> The compiler is free to generate vectorized operations for zero'ing > >> memory. The kernel does not use the vector unit on RISCV, similar to > >> architectures such as x86 where we use `-mno-mmx` et al to prevent the > >> implicit vectorization. Perform a similar check for > >> `-mno-implicit-float` to avoid this on RISC-V targets. > > > > I'm not sure if we should be emitting either of the vector or floating > > point instrucitons in the kernel without explicitly marking the section > > of code which is using them such as specific accelerator blocks. > > Yep, we can't let the compiler just blindly enable V or F/D. V would > very much break things as we have no support, but even when that's in > we'll we at roughly the same spot as F/D are now where we need to handle > the lazy save/restore bits. > > This looks like an LLVM-only option, I see at least some handling here > > https://github.com/llvm/llvm-project/blob/a72883b7612f5c00b592da85ed2f1fd81258cc08/clang/lib/Driver/ToolChains/Clang.cpp#L2098 > > but I don't really know LLVM enough to understand if there's some > default for `-mimplicit-float` and I can't find anything in the docs. > If it can be turned on by default and that results in F/D/V instructions > then we'll need to explicitly turn it off, and that would need to be > backported. Yes, this is an LLVM option, but I think that the `cc-option` wrapping should help ensure that we do not break the gcc build. This only recently was added to clang, so an older clang would also miss this flag. The `-mimplicit-float` is the default AFAIK, which is why we needed to add this flag in the first place. Enabling V exposed this, which is why the commit message mentions vector. > > Maybe Nick or Nathan knows what's up here?
On 16 December 2022 12:56:23 GMT-08:00, Saleem Abdulrasool <abdulras@google.com> wrote: >On Fri, Dec 16, 2022 at 11:54 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: >> >> On Fri, 16 Dec 2022 11:45:21 PST (-0800), ben.dooks@codethink.co.uk wrote: >> > >> > >> > On 2022-12-16 18:50, Saleem Abdulrasool wrote: >> >> The compiler is free to generate vectorized operations for zero'ing >> >> memory. The kernel does not use the vector unit on RISCV, similar to >> >> architectures such as x86 where we use `-mno-mmx` et al to prevent the >> >> implicit vectorization. Perform a similar check for >> >> `-mno-implicit-float` to avoid this on RISC-V targets. >> > >> > I'm not sure if we should be emitting either of the vector or floating >> > point instrucitons in the kernel without explicitly marking the section >> > of code which is using them such as specific accelerator blocks. >> >> Yep, we can't let the compiler just blindly enable V or F/D. V would >> very much break things as we have no support, but even when that's in >> we'll we at roughly the same spot as F/D are now where we need to handle >> the lazy save/restore bits. >> >> This looks like an LLVM-only option, I see at least some handling here >> >> https://github.com/llvm/llvm-project/blob/a72883b7612f5c00b592da85ed2f1fd81258cc08/clang/lib/Driver/ToolChains/Clang.cpp#L2098 >> >> but I don't really know LLVM enough to understand if there's some >> default for `-mimplicit-float` and I can't find anything in the docs. >> If it can be turned on by default and that results in F/D/V instructions >> then we'll need to explicitly turn it off, and that would need to be >> backported. > >Yes, this is an LLVM option, but I think that the `cc-option` wrapping >should help ensure that we do not break the gcc build. This only >recently was added to clang, so an older clang would also miss this >flag. The `-mimplicit-float` is the default AFAIK, which is why we >needed to add this flag in the first place. Enabling V exposed this, >which is why the commit message mentions vector. You've said "enabling V" in the comment and here. By that, do you mean when V support is enabled in clang or when it is enabled in Linux?
On Fri, Dec 16, 2022 at 6:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > On 16 December 2022 12:56:23 GMT-08:00, Saleem Abdulrasool <abdulras@google.com> wrote: > >On Fri, Dec 16, 2022 at 11:54 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > >> > >> On Fri, 16 Dec 2022 11:45:21 PST (-0800), ben.dooks@codethink.co.uk wrote: > >> > > >> > > >> > On 2022-12-16 18:50, Saleem Abdulrasool wrote: > >> >> The compiler is free to generate vectorized operations for zero'ing > >> >> memory. The kernel does not use the vector unit on RISCV, similar to > >> >> architectures such as x86 where we use `-mno-mmx` et al to prevent the > >> >> implicit vectorization. Perform a similar check for > >> >> `-mno-implicit-float` to avoid this on RISC-V targets. > >> > > >> > I'm not sure if we should be emitting either of the vector or floating > >> > point instrucitons in the kernel without explicitly marking the section > >> > of code which is using them such as specific accelerator blocks. > >> > >> Yep, we can't let the compiler just blindly enable V or F/D. V would > >> very much break things as we have no support, but even when that's in > >> we'll we at roughly the same spot as F/D are now where we need to handle > >> the lazy save/restore bits. > >> > >> This looks like an LLVM-only option, I see at least some handling here > >> > >> https://github.com/llvm/llvm-project/blob/a72883b7612f5c00b592da85ed2f1fd81258cc08/clang/lib/Driver/ToolChains/Clang.cpp#L2098 > >> > >> but I don't really know LLVM enough to understand if there's some > >> default for `-mimplicit-float` and I can't find anything in the docs. > >> If it can be turned on by default and that results in F/D/V instructions > >> then we'll need to explicitly turn it off, and that would need to be > >> backported. > > > >Yes, this is an LLVM option, but I think that the `cc-option` wrapping > >should help ensure that we do not break the gcc build. This only > >recently was added to clang, so an older clang would also miss this > >flag. The `-mimplicit-float` is the default AFAIK, which is why we > >needed to add this flag in the first place. Enabling V exposed this, > >which is why the commit message mentions vector. > > You've said "enabling V" in the comment and here. > By that, do you mean when V support is enabled in clang or when it is enabled in Linux? Excellent distinction. I meant enabled in the compiler, enabling it in the kernel is not yet possible without the pending patchset. This makes us robust to when that patchset is merged, but in the meantime, this protects against the V extension being enabled in the toolchain.
On Mon, Dec 19, 2022 at 07:21:32AM -0800, Saleem Abdulrasool wrote: > On Fri, Dec 16, 2022 at 6:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > On 16 December 2022 12:56:23 GMT-08:00, Saleem Abdulrasool <abdulras@google.com> wrote: > > >On Fri, Dec 16, 2022 at 11:54 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > >> > > >> On Fri, 16 Dec 2022 11:45:21 PST (-0800), ben.dooks@codethink.co.uk wrote: > > >> > > > >> > > > >> > On 2022-12-16 18:50, Saleem Abdulrasool wrote: > > >> >> The compiler is free to generate vectorized operations for zero'ing > > >> >> memory. The kernel does not use the vector unit on RISCV, similar to > > >> >> architectures such as x86 where we use `-mno-mmx` et al to prevent the > > >> >> implicit vectorization. Perform a similar check for > > >> >> `-mno-implicit-float` to avoid this on RISC-V targets. > > >> > > > >> > I'm not sure if we should be emitting either of the vector or floating > > >> > point instrucitons in the kernel without explicitly marking the section > > >> > of code which is using them such as specific accelerator blocks. > > >> > > >> Yep, we can't let the compiler just blindly enable V or F/D. V would > > >> very much break things as we have no support, but even when that's in > > >> we'll we at roughly the same spot as F/D are now where we need to handle > > >> the lazy save/restore bits. > > >> > > >> This looks like an LLVM-only option, I see at least some handling here > > >> > > >> https://github.com/llvm/llvm-project/blob/a72883b7612f5c00b592da85ed2f1fd81258cc08/clang/lib/Driver/ToolChains/Clang.cpp#L2098 > > >> > > >> but I don't really know LLVM enough to understand if there's some > > >> default for `-mimplicit-float` and I can't find anything in the docs. > > >> If it can be turned on by default and that results in F/D/V instructions > > >> then we'll need to explicitly turn it off, and that would need to be > > >> backported. > > > > > >Yes, this is an LLVM option, but I think that the `cc-option` wrapping > > >should help ensure that we do not break the gcc build. This only > > >recently was added to clang, so an older clang would also miss this > > >flag. The `-mimplicit-float` is the default AFAIK, which is why we > > >needed to add this flag in the first place. Enabling V exposed this, > > >which is why the commit message mentions vector. > > > > You've said "enabling V" in the comment and here. > > By that, do you mean when V support is enabled in clang or when it is enabled in Linux? > > Excellent distinction. I meant enabled in the compiler, enabling it > in the kernel is not yet possible without the pending patchset. This > makes us robust to when that patchset is merged, but in the meantime, > this protects against the V extension being enabled in the toolchain. Ah cool. I figured that it was not possible without the vector patchset but I was not 100% as it was a wee bit vague ;) Since V will not be enabled without that patchset, I guess this does not *have* to go as a fix or to stable? Per the option's name & https://github.com/llvm/llvm-project/commit/549231d38e10de7371adb85f5452d42ad42f4201 it may however be better to backport it anyway, in case implicit use of fp registers does arrive. You mentioned the gcc build & gcc-12 is fine: https://patchwork.kernel.org/project/linux-riscv/patch/20221216185012.2342675-1-abdulras@google.com/ Anyway, seems like a sensible addition to me, so: Reviewed-by: Conor Dooley <conor.dooley@microchip.com> I think it may also be good to do: Link: https://github.com/llvm/llvm-project/commit/549231d38e10de7371adb85f5452d42ad42f4201 Thanks, Conor.
Hi, On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > The compiler is free to generate vectorized operations for zero'ing > memory. The kernel does not use the vector unit on RISCV, similar to > architectures such as x86 where we use `-mno-mmx` et al to prevent the > implicit vectorization. Perform a similar check for > `-mno-implicit-float` to avoid this on RISC-V targets. > > Signed-off-by: Saleem Abdulrasool <abdulras@google.com> > --- > arch/riscv/Makefile | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 0d13b597cb55..68433476a96e 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -89,6 +89,10 @@ KBUILD_AFLAGS_MODULE += $(call as-option,-Wa$(comma)-mno-relax) > # architectures. It's faster to have GCC emit only aligned accesses. > KBUILD_CFLAGS += $(call cc-option,-mstrict-align) > > +# Ensure that we do not vectorize the kernel code when the `v` extension is > +# enabled. This mirrors the `-mno-mmx` et al on x86. > +KBUILD_CFLAGS += $(call cc-option,-mno-implicit-float) This looks like an LLVM flag, but not GCC. Can you elaborate what exact combination (compiler flag and source) would cause an issue? From your description, I guess it's that when enabling V extension in LLVM, the compiler tries to use vector instructions to zero memory, correct? Can you confirm LLVM does not emit any float instructions (like F/D extensions) because the flag name suggests something like "float"? > + > ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y) > prepare: stack_protector_prepare > stack_protector_prepare: prepare0 > -- Regards, Bin
On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi, > > On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > > > The compiler is free to generate vectorized operations for zero'ing > > memory. The kernel does not use the vector unit on RISCV, similar to > > architectures such as x86 where we use `-mno-mmx` et al to prevent the > > implicit vectorization. Perform a similar check for > > `-mno-implicit-float` to avoid this on RISC-V targets. > > > > Signed-off-by: Saleem Abdulrasool <abdulras@google.com> > > --- > > arch/riscv/Makefile | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > index 0d13b597cb55..68433476a96e 100644 > > --- a/arch/riscv/Makefile > > +++ b/arch/riscv/Makefile > > @@ -89,6 +89,10 @@ KBUILD_AFLAGS_MODULE += $(call as-option,-Wa$(comma)-mno-relax) > > # architectures. It's faster to have GCC emit only aligned accesses. > > KBUILD_CFLAGS += $(call cc-option,-mstrict-align) > > > > +# Ensure that we do not vectorize the kernel code when the `v` extension is > > +# enabled. This mirrors the `-mno-mmx` et al on x86. > > +KBUILD_CFLAGS += $(call cc-option,-mno-implicit-float) > > This looks like an LLVM flag, but not GCC. Correct, this is a clang flag, though I imagine that GCC will need a similar flag once it receives support for the V extension. > Can you elaborate what exact combination (compiler flag and source) > would cause an issue? The particular case that I was using was simply `clang -target riscv64-unknown-linux-musl -march=rv64gcv` off of main. > From your description, I guess it's that when enabling V extension in > LLVM, the compiler tries to use vector instructions to zero memory, > correct? Correct. > Can you confirm LLVM does not emit any float instructions (like F/D > extensions) because the flag name suggests something like "float"? The `-mno-implicit-float` should disable any such emission. I assume that you are worried about the case without the flag? I'm not 100% certain without this flag, but the RISCV build with this flag has been running smoothly locally for a while. > > + > > ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y) > > prepare: stack_protector_prepare > > stack_protector_prepare: prepare0 > > -- > > Regards, > Bin
Hi, On Thu, Dec 22, 2022 at 1:39 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi, > > > > On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > > > > > The compiler is free to generate vectorized operations for zero'ing > > > memory. The kernel does not use the vector unit on RISCV, similar to > > > architectures such as x86 where we use `-mno-mmx` et al to prevent the > > > implicit vectorization. Perform a similar check for > > > `-mno-implicit-float` to avoid this on RISC-V targets. > > > > > > Signed-off-by: Saleem Abdulrasool <abdulras@google.com> > > > --- > > > arch/riscv/Makefile | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > index 0d13b597cb55..68433476a96e 100644 > > > --- a/arch/riscv/Makefile > > > +++ b/arch/riscv/Makefile > > > @@ -89,6 +89,10 @@ KBUILD_AFLAGS_MODULE += $(call as-option,-Wa$(comma)-mno-relax) > > > # architectures. It's faster to have GCC emit only aligned accesses. > > > KBUILD_CFLAGS += $(call cc-option,-mstrict-align) > > > > > > +# Ensure that we do not vectorize the kernel code when the `v` extension is > > > +# enabled. This mirrors the `-mno-mmx` et al on x86. > > > +KBUILD_CFLAGS += $(call cc-option,-mno-implicit-float) > > > > This looks like an LLVM flag, but not GCC. > > Correct, this is a clang flag, though I imagine that GCC will need a > similar flag once it receives support for the V extension. > > > Can you elaborate what exact combination (compiler flag and source) > > would cause an issue? > > The particular case that I was using was simply `clang -target > riscv64-unknown-linux-musl -march=rv64gcv` off of main. > > > From your description, I guess it's that when enabling V extension in > > LLVM, the compiler tries to use vector instructions to zero memory, > > correct? > > Correct. Thanks for the confirmation. > > > Can you confirm LLVM does not emit any float instructions (like F/D > > extensions) because the flag name suggests something like "float"? > > The `-mno-implicit-float` should disable any such emission. I assume > that you are worried about the case without the flag? I'm not 100% > certain without this flag, but the RISCV build with this flag has been > running smoothly locally for a while. > > I still have some questions about the `-mno-implicit-float` option's behavior. - If this option is not on, does the compiler emit any F/D extension instruction for zero'ing memory when -march=rv64g? I want to know whether the `-mno-implicit-float` option only takes effect when "v" appears on the -march string. - If the answer to the above question is no, I wonder why the option is called `-mno-implicit-float` as float suggests the FPU usage, but actually it is about vectorization. The Clang documentation says almost nothing about this option. > > > + > > > ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y) > > > prepare: stack_protector_prepare > > > stack_protector_prepare: prepare0 > > > -- Regards, Bin
On Thu, Dec 22, 2022 at 1:41 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi, > > On Thu, Dec 22, 2022 at 1:39 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > > > On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi, > > > > > > On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > > > > > > > The compiler is free to generate vectorized operations for zero'ing > > > > memory. The kernel does not use the vector unit on RISCV, similar to > > > > architectures such as x86 where we use `-mno-mmx` et al to prevent the > > > > implicit vectorization. Perform a similar check for > > > > `-mno-implicit-float` to avoid this on RISC-V targets. > > > > > > > > Signed-off-by: Saleem Abdulrasool <abdulras@google.com> > > > > --- > > > > arch/riscv/Makefile | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > > index 0d13b597cb55..68433476a96e 100644 > > > > --- a/arch/riscv/Makefile > > > > +++ b/arch/riscv/Makefile > > > > @@ -89,6 +89,10 @@ KBUILD_AFLAGS_MODULE += $(call as-option,-Wa$(comma)-mno-relax) > > > > # architectures. It's faster to have GCC emit only aligned accesses. > > > > KBUILD_CFLAGS += $(call cc-option,-mstrict-align) > > > > > > > > +# Ensure that we do not vectorize the kernel code when the `v` extension is > > > > +# enabled. This mirrors the `-mno-mmx` et al on x86. > > > > +KBUILD_CFLAGS += $(call cc-option,-mno-implicit-float) > > > > > > This looks like an LLVM flag, but not GCC. > > > > Correct, this is a clang flag, though I imagine that GCC will need a > > similar flag once it receives support for the V extension. > > > > > Can you elaborate what exact combination (compiler flag and source) > > > would cause an issue? > > > > The particular case that I was using was simply `clang -target > > riscv64-unknown-linux-musl -march=rv64gcv` off of main. > > > > > From your description, I guess it's that when enabling V extension in > > > LLVM, the compiler tries to use vector instructions to zero memory, > > > correct? > > > > Correct. > > Thanks for the confirmation. > > > > > > Can you confirm LLVM does not emit any float instructions (like F/D > > > extensions) because the flag name suggests something like "float"? > > > > The `-mno-implicit-float` should disable any such emission. I assume > > that you are worried about the case without the flag? I'm not 100% > > certain without this flag, but the RISCV build with this flag has been > > running smoothly locally for a while. > > > > > > I still have some questions about the `-mno-implicit-float` option's behavior. > > - If this option is not on, does the compiler emit any F/D extension > instruction for zero'ing memory when -march=rv64g? I want to know > whether the `-mno-implicit-float` option only takes effect when "v" > appears on the -march string. AFAIK, and from a quick test, no, it will not. That also makes sense since the F/D/Q handling is not as likely to be useful for generating a 0-filled array. No, the use of `-mno-implicit-float` is not guarded by the use of the vector extension, but it does only impact the vectorized code generation (the loop vectorizer, load/store vectorizer, and SLP vectorizer). > - If the answer to the above question is no, I wonder why the option > is called `-mno-implicit-float` as float suggests the FPU usage, but > actually it is about vectorization. The Clang documentation says > almost nothing about this option. The flag itself is from GCC, it was added for the ARM architecture, to prefer using the scalar core over the VFP register set as ARM uses the VFP for vectorized operations. As it so happens, internally in LLVM, the loop vectorizer uses the (internal) `NoImplicitFloat` function attribute to prevent the loop from being vectorized, and the flag that controls this is exposed as `-mimplicit-float` and `-mno-implicit-float`. > > > > + > > > > ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y) > > > > prepare: stack_protector_prepare > > > > stack_protector_prepare: prepare0 > > > > -- > > Regards, > Bin
Hi, On Thu, Dec 22, 2022 at 11:23 PM Saleem Abdulrasool <abdulras@google.com> wrote: > > On Thu, Dec 22, 2022 at 1:41 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi, > > > > On Thu, Dec 22, 2022 at 1:39 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > > > > > On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > Hi, > > > > > > > > On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > > > > > > > > > The compiler is free to generate vectorized operations for zero'ing > > > > > memory. The kernel does not use the vector unit on RISCV, similar to > > > > > architectures such as x86 where we use `-mno-mmx` et al to prevent the > > > > > implicit vectorization. Perform a similar check for > > > > > `-mno-implicit-float` to avoid this on RISC-V targets. > > > > > > > > > > Signed-off-by: Saleem Abdulrasool <abdulras@google.com> > > > > > --- > > > > > arch/riscv/Makefile | 4 ++++ > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > > > index 0d13b597cb55..68433476a96e 100644 > > > > > --- a/arch/riscv/Makefile > > > > > +++ b/arch/riscv/Makefile > > > > > @@ -89,6 +89,10 @@ KBUILD_AFLAGS_MODULE += $(call as-option,-Wa$(comma)-mno-relax) > > > > > # architectures. It's faster to have GCC emit only aligned accesses. > > > > > KBUILD_CFLAGS += $(call cc-option,-mstrict-align) > > > > > > > > > > +# Ensure that we do not vectorize the kernel code when the `v` extension is > > > > > +# enabled. This mirrors the `-mno-mmx` et al on x86. > > > > > +KBUILD_CFLAGS += $(call cc-option,-mno-implicit-float) > > > > > > > > This looks like an LLVM flag, but not GCC. > > > > > > Correct, this is a clang flag, though I imagine that GCC will need a > > > similar flag once it receives support for the V extension. > > > > > > > Can you elaborate what exact combination (compiler flag and source) > > > > would cause an issue? > > > > > > The particular case that I was using was simply `clang -target > > > riscv64-unknown-linux-musl -march=rv64gcv` off of main. > > > > > > > From your description, I guess it's that when enabling V extension in > > > > LLVM, the compiler tries to use vector instructions to zero memory, > > > > correct? > > > > > > Correct. > > > > Thanks for the confirmation. > > > > > > > > > Can you confirm LLVM does not emit any float instructions (like F/D > > > > extensions) because the flag name suggests something like "float"? > > > > > > The `-mno-implicit-float` should disable any such emission. I assume > > > that you are worried about the case without the flag? I'm not 100% > > > certain without this flag, but the RISCV build with this flag has been > > > running smoothly locally for a while. > > > > > > > > > > I still have some questions about the `-mno-implicit-float` option's behavior. > > > > - If this option is not on, does the compiler emit any F/D extension > > instruction for zero'ing memory when -march=rv64g? I want to know > > whether the `-mno-implicit-float` option only takes effect when "v" > > appears on the -march string. > > AFAIK, and from a quick test, no, it will not. That also makes sense > since the F/D/Q handling is not as likely to be useful for generating > a 0-filled array. No, the use of `-mno-implicit-float` is not guarded > by the use of the vector extension, but it does only impact the > vectorized code generation (the loop vectorizer, load/store > vectorizer, and SLP vectorizer). Thank you. The quick test you did seems to match what the LLVM commit [1] says: "It also disables implicit uses of scalar FP, but I don't know if we have any of those for RISC-V." [1] https://github.com/llvm/llvm-project/commit/549231d38e10de7371adb85f5452d42ad42f4201 > > > - If the answer to the above question is no, I wonder why the option > > is called `-mno-implicit-float` as float suggests the FPU usage, but > > actually it is about vectorization. The Clang documentation says > > almost nothing about this option. > > The flag itself is from GCC, it was added for the ARM architecture, to > prefer using the scalar core over the VFP register set as ARM uses the > VFP for vectorized operations. As it so happens, internally in LLVM, > the loop vectorizer uses the (internal) `NoImplicitFloat` function > attribute to prevent the loop from being vectorized, and the flag that > controls this is exposed as `-mimplicit-float` and > `-mno-implicit-float`. > It seems GCC does not have such a flag. Thanks for the history introduction. It was introduced on Arm to disable vectorized operation using VFP, hence it was named as -no-implict-float. But IMHO the option is badly named. Maybe -no-implicit-vectorization better fits what it really does. FWIW, Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Regards, Bin
On Thu, Dec 22, 2022 at 10:57 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi, > > On Thu, Dec 22, 2022 at 11:23 PM Saleem Abdulrasool <abdulras@google.com> wrote: > > > > On Thu, Dec 22, 2022 at 1:41 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi, > > > > > > On Thu, Dec 22, 2022 at 1:39 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > > > > > > > On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <abdulras@google.com> wrote: > > > > > > > > > > > > The compiler is free to generate vectorized operations for zero'ing > > > > > > memory. The kernel does not use the vector unit on RISCV, similar to > > > > > > architectures such as x86 where we use `-mno-mmx` et al to prevent the > > > > > > implicit vectorization. Perform a similar check for > > > > > > `-mno-implicit-float` to avoid this on RISC-V targets. > > > > > > > > > > > > Signed-off-by: Saleem Abdulrasool <abdulras@google.com> > > > > > > --- > > > > > > arch/riscv/Makefile | 4 ++++ > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > > > > index 0d13b597cb55..68433476a96e 100644 > > > > > > --- a/arch/riscv/Makefile > > > > > > +++ b/arch/riscv/Makefile > > > > > > @@ -89,6 +89,10 @@ KBUILD_AFLAGS_MODULE += $(call as-option,-Wa$(comma)-mno-relax) > > > > > > # architectures. It's faster to have GCC emit only aligned accesses. > > > > > > KBUILD_CFLAGS += $(call cc-option,-mstrict-align) > > > > > > > > > > > > +# Ensure that we do not vectorize the kernel code when the `v` extension is > > > > > > +# enabled. This mirrors the `-mno-mmx` et al on x86. > > > > > > +KBUILD_CFLAGS += $(call cc-option,-mno-implicit-float) > > > > > > > > > > This looks like an LLVM flag, but not GCC. > > > > > > > > Correct, this is a clang flag, though I imagine that GCC will need a > > > > similar flag once it receives support for the V extension. > > > > > > > > > Can you elaborate what exact combination (compiler flag and source) > > > > > would cause an issue? > > > > > > > > The particular case that I was using was simply `clang -target > > > > riscv64-unknown-linux-musl -march=rv64gcv` off of main. > > > > > > > > > From your description, I guess it's that when enabling V extension in > > > > > LLVM, the compiler tries to use vector instructions to zero memory, > > > > > correct? > > > > > > > > Correct. > > > > > > Thanks for the confirmation. > > > > > > > > > > > > Can you confirm LLVM does not emit any float instructions (like F/D > > > > > extensions) because the flag name suggests something like "float"? > > > > > > > > The `-mno-implicit-float` should disable any such emission. I assume > > > > that you are worried about the case without the flag? I'm not 100% > > > > certain without this flag, but the RISCV build with this flag has been > > > > running smoothly locally for a while. > > > > > > > > > > > > > > I still have some questions about the `-mno-implicit-float` option's behavior. > > > > > > - If this option is not on, does the compiler emit any F/D extension > > > instruction for zero'ing memory when -march=rv64g? I want to know > > > whether the `-mno-implicit-float` option only takes effect when "v" > > > appears on the -march string. > > > > AFAIK, and from a quick test, no, it will not. That also makes sense > > since the F/D/Q handling is not as likely to be useful for generating > > a 0-filled array. No, the use of `-mno-implicit-float` is not guarded > > by the use of the vector extension, but it does only impact the > > vectorized code generation (the loop vectorizer, load/store > > vectorizer, and SLP vectorizer). > > Thank you. The quick test you did seems to match what the LLVM commit [1] says: > > "It also disables implicit uses of scalar FP, but I don't know if > we have any of those for RISC-V." > > [1] https://github.com/llvm/llvm-project/commit/549231d38e10de7371adb85f5452d42ad42f4201 > > > > > > - If the answer to the above question is no, I wonder why the option > > > is called `-mno-implicit-float` as float suggests the FPU usage, but > > > actually it is about vectorization. The Clang documentation says > > > almost nothing about this option. > > > > The flag itself is from GCC, it was added for the ARM architecture, to > > prefer using the scalar core over the VFP register set as ARM uses the > > VFP for vectorized operations. As it so happens, internally in LLVM, > > the loop vectorizer uses the (internal) `NoImplicitFloat` function > > attribute to prevent the loop from being vectorized, and the flag that > > controls this is exposed as `-mimplicit-float` and > > `-mno-implicit-float`. > > > > It seems GCC does not have such a flag. Thanks for the history > introduction. It was introduced on Arm to disable vectorized operation > using VFP, hence it was named as -no-implict-float. But IMHO the > option is badly named. Maybe -no-implicit-vectorization better fits > what it really does. The option is present on ARM GCC, but not RISC-V GCC. Sure, the option could be better named - personally, I'd prefer `-mgeneral-regs-only` to match the x86 convention which leaves it sufficiently generalised that future extensions would easily fit into the behavioural control. Much like the Linux kernel's prime directive: "we do not break userspace'', the LLVM toolchain has a similar view point: options which have shipped are considered permanent. Even if renamed, it would be an alias and the old option sticks around in near perpetuity. > FWIW, > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > Regards, > Bin
On Fri, 16 Dec 2022 10:50:12 PST (-0800), abdulras@google.com wrote: > The compiler is free to generate vectorized operations for zero'ing > memory. The kernel does not use the vector unit on RISCV, similar to > architectures such as x86 where we use `-mno-mmx` et al to prevent the > implicit vectorization. Perform a similar check for > `-mno-implicit-float` to avoid this on RISC-V targets. > > Signed-off-by: Saleem Abdulrasool <abdulras@google.com> > --- > arch/riscv/Makefile | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 0d13b597cb55..68433476a96e 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -89,6 +89,10 @@ KBUILD_AFLAGS_MODULE += $(call as-option,-Wa$(comma)-mno-relax) > # architectures. It's faster to have GCC emit only aligned accesses. > KBUILD_CFLAGS += $(call cc-option,-mstrict-align) > > +# Ensure that we do not vectorize the kernel code when the `v` extension is > +# enabled. This mirrors the `-mno-mmx` et al on x86. > +KBUILD_CFLAGS += $(call cc-option,-mno-implicit-float) > + > ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y) > prepare: stack_protector_prepare > stack_protector_prepare: prepare0 Sorry to just restart the thread, but there's been discussions on this in a bunch of places. From my understanding, we don't actually need this: we have this tricky line in the Makefile KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y)) that removes the floating-point extenions from what the kernel is built with, so adding `-mno-implicit-float` doesn't actually do anything (and we'd end up with essentially the same thing for V when it gets added). So unless I'm missing something, we don't need this.
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 0d13b597cb55..68433476a96e 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -89,6 +89,10 @@ KBUILD_AFLAGS_MODULE += $(call as-option,-Wa$(comma)-mno-relax) # architectures. It's faster to have GCC emit only aligned accesses. KBUILD_CFLAGS += $(call cc-option,-mstrict-align) +# Ensure that we do not vectorize the kernel code when the `v` extension is +# enabled. This mirrors the `-mno-mmx` et al on x86. +KBUILD_CFLAGS += $(call cc-option,-mno-implicit-float) + ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y) prepare: stack_protector_prepare stack_protector_prepare: prepare0
The compiler is free to generate vectorized operations for zero'ing memory. The kernel does not use the vector unit on RISCV, similar to architectures such as x86 where we use `-mno-mmx` et al to prevent the implicit vectorization. Perform a similar check for `-mno-implicit-float` to avoid this on RISC-V targets. Signed-off-by: Saleem Abdulrasool <abdulras@google.com> --- arch/riscv/Makefile | 4 ++++ 1 file changed, 4 insertions(+)