2022-12-16 19:20:50

by Saleem Abdulrasool

[permalink] [raw]
Subject: [PATCH] riscv: avoid enabling vectorized code generation

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 <[email protected]>
---
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
--
2.39.0.314.g84b9a713c41-goog


2022-12-16 19:22:22

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

Hey Saleem,

On 16 December 2022 10:50:12 GMT-08:00, Saleem Abdulrasool <[email protected]> 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 <[email protected]>
>---
> 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/[email protected]/

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

2022-12-16 19:50:38

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation



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.

--
Ben

2022-12-16 20:16:53

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

On Fri, 16 Dec 2022 11:45:21 PST (-0800), [email protected] 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?

2022-12-16 21:16:07

by Saleem Abdulrasool

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

On Fri, Dec 16, 2022 at 11:54 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Fri, 16 Dec 2022 11:45:21 PST (-0800), [email protected] 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?

2022-12-17 02:32:09

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation



On 16 December 2022 12:56:23 GMT-08:00, Saleem Abdulrasool <[email protected]> wrote:
>On Fri, Dec 16, 2022 at 11:54 AM Palmer Dabbelt <[email protected]> wrote:
>>
>> On Fri, 16 Dec 2022 11:45:21 PST (-0800), [email protected] 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?

2022-12-19 15:58:29

by Saleem Abdulrasool

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

On Fri, Dec 16, 2022 at 6:02 PM Conor Dooley <[email protected]> wrote:
>
>
>
> On 16 December 2022 12:56:23 GMT-08:00, Saleem Abdulrasool <[email protected]> wrote:
> >On Fri, Dec 16, 2022 at 11:54 AM Palmer Dabbelt <[email protected]> wrote:
> >>
> >> On Fri, 16 Dec 2022 11:45:21 PST (-0800), [email protected] 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.

2022-12-19 17:09:51

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

On Mon, Dec 19, 2022 at 07:21:32AM -0800, Saleem Abdulrasool wrote:
> On Fri, Dec 16, 2022 at 6:02 PM Conor Dooley <[email protected]> wrote:
> >
> >
> >
> > On 16 December 2022 12:56:23 GMT-08:00, Saleem Abdulrasool <[email protected]> wrote:
> > >On Fri, Dec 16, 2022 at 11:54 AM Palmer Dabbelt <[email protected]> wrote:
> > >>
> > >> On Fri, 16 Dec 2022 11:45:21 PST (-0800), [email protected] 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/[email protected]/

Anyway, seems like a sensible addition to me, so:
Reviewed-by: Conor Dooley <[email protected]>
I think it may also be good to do:
Link: https://github.com/llvm/llvm-project/commit/549231d38e10de7371adb85f5452d42ad42f4201

Thanks,
Conor.


Attachments:
(No filename) (3.62 kB)
signature.asc (235.00 B)
Download all attachments

2022-12-21 16:32:22

by Bin Meng

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

Hi,

On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <[email protected]> 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 <[email protected]>
> ---
> 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

2022-12-21 17:45:30

by Saleem Abdulrasool

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <[email protected]> wrote:
>
> Hi,
>
> On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <[email protected]> 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 <[email protected]>
> > ---
> > 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

2022-12-22 10:39:14

by Bin Meng

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

Hi,

On Thu, Dec 22, 2022 at 1:39 AM Saleem Abdulrasool <[email protected]> wrote:
>
> On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <[email protected]> wrote:
> >
> > Hi,
> >
> > On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <[email protected]> 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 <[email protected]>
> > > ---
> > > 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

2022-12-22 15:45:53

by Saleem Abdulrasool

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

On Thu, Dec 22, 2022 at 1:41 AM Bin Meng <[email protected]> wrote:
>
> Hi,
>
> On Thu, Dec 22, 2022 at 1:39 AM Saleem Abdulrasool <[email protected]> wrote:
> >
> > On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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

2022-12-23 07:22:05

by Bin Meng

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

Hi,

On Thu, Dec 22, 2022 at 11:23 PM Saleem Abdulrasool <[email protected]> wrote:
>
> On Thu, Dec 22, 2022 at 1:41 AM Bin Meng <[email protected]> wrote:
> >
> > Hi,
> >
> > On Thu, Dec 22, 2022 at 1:39 AM Saleem Abdulrasool <[email protected]> wrote:
> > >
> > > On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <[email protected]> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <[email protected]> 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 <[email protected]>
> > > > > ---
> > > > > 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 <[email protected]>

Regards,
Bin

2022-12-27 15:27:03

by Saleem Abdulrasool

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

On Thu, Dec 22, 2022 at 10:57 PM Bin Meng <[email protected]> wrote:
>
> Hi,
>
> On Thu, Dec 22, 2022 at 11:23 PM Saleem Abdulrasool <[email protected]> wrote:
> >
> > On Thu, Dec 22, 2022 at 1:41 AM Bin Meng <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Dec 22, 2022 at 1:39 AM Saleem Abdulrasool <[email protected]> wrote:
> > > >
> > > > On Wed, Dec 21, 2022 at 8:17 AM Bin Meng <[email protected]> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Sat, Dec 17, 2022 at 3:12 AM Saleem Abdulrasool <[email protected]> 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 <[email protected]>
> > > > > > ---
> > > > > > 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 <[email protected]>
>
> Regards,
> Bin

2023-02-08 17:56:13

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] riscv: avoid enabling vectorized code generation

On Fri, 16 Dec 2022 10:50:12 PST (-0800), [email protected] 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 <[email protected]>
> ---
> 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.