Re: [PATCH] riscv: avoid enabling vectorized code generation

[Saleem Abdulrasool]

On Fri, Dec 16, 2022 at 6:02 PM Conor Dooley <conor@xxxxxxxxxx> wrote:
>
>
>
> On 16 December 2022 12:56:23 GMT-08:00, Saleem Abdulrasool <abdulras@xxxxxxxxxx> wrote:
> >On Fri, Dec 16, 2022 at 11:54 AM Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote:
> >>
> >> On Fri, 16 Dec 2022 11:45:21 PST (-0800), ben.dooks@xxxxxxxxxxxxxxx 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.