| Message ID | 20240411-dev-charlie-support_thead_vector_6_9-v1-7-4af9815ec746@rivosinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: Support vendor extensions and xtheadvector | expand |
On Thu, Apr 11, 2024 at 09:11:13PM -0700, Charlie Jenkins wrote: > When alternatives are disabled, riscv_cpu_isa_extension_(un)likely() > checks if the current cpu supports the selected extension if not all > cpus support the extension. It is sufficient to only check if the > current cpu supports the extension. > > The alternatives code to handle if all cpus support an extension is > factored out into a new function to support this. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > --- > static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext) > { > - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext)) > - return true; > + compiletime_assert(ext < RISCV_ISA_EXT_MAX, > + "ext must be < RISCV_ISA_EXT_MAX"); > > - return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && __riscv_has_extension_unlikely_alternatives(ext)) > + return true; > + else > + return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > } static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext) { if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext)) return true; return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); } This is the code as things stand. If alternatives are disabled, the if statement becomes if (0 && foo) which will lead to the function call getting constant folded away and all you end up with is the call to __riscv_isa_extension_available(). Unless I am missing something, I don't think this patch has any affect? Thanks, Conor.
On Fri, Apr 12, 2024 at 11:40:38AM +0100, Conor Dooley wrote: > On Thu, Apr 11, 2024 at 09:11:13PM -0700, Charlie Jenkins wrote: > > When alternatives are disabled, riscv_cpu_isa_extension_(un)likely() > > checks if the current cpu supports the selected extension if not all > > cpus support the extension. It is sufficient to only check if the > > current cpu supports the extension. > > > > The alternatives code to handle if all cpus support an extension is > > factored out into a new function to support this. > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > --- > > > static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext) > > { > > - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext)) > > - return true; > > + compiletime_assert(ext < RISCV_ISA_EXT_MAX, > > + "ext must be < RISCV_ISA_EXT_MAX"); > > > > - return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && __riscv_has_extension_unlikely_alternatives(ext)) > > + return true; > > + else > > + return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > > } > > static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext) > { > if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext)) > return true; > > return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > } > > This is the code as things stand. If alternatives are disabled, the if > statement becomes if (0 && foo) which will lead to the function call > getting constant folded away and all you end up with is the call to > __riscv_isa_extension_available(). Unless I am missing something, I don't > think this patch has any affect? Yeah I fumbled this one it appears. I got thrown off by the nested IS_ENABLED(CONFIG_RISCV_ALTERNATIVE). This patch eliminates the need for this and maybe can avoid avoid confusion in the future. - Charlie > > Thanks, > Conor. >
On Fri, Apr 12, 2024 at 10:34:28AM -0700, Charlie Jenkins wrote: > On Fri, Apr 12, 2024 at 11:40:38AM +0100, Conor Dooley wrote: > > On Thu, Apr 11, 2024 at 09:11:13PM -0700, Charlie Jenkins wrote: > > > When alternatives are disabled, riscv_cpu_isa_extension_(un)likely() > > > checks if the current cpu supports the selected extension if not all > > > cpus support the extension. It is sufficient to only check if the > > > current cpu supports the extension. > > > > > > The alternatives code to handle if all cpus support an extension is > > > factored out into a new function to support this. > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > --- > > > > > static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext) > > > { > > > - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext)) > > > - return true; > > > + compiletime_assert(ext < RISCV_ISA_EXT_MAX, > > > + "ext must be < RISCV_ISA_EXT_MAX"); > > > > > > - return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > > > + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && __riscv_has_extension_unlikely_alternatives(ext)) > > > + return true; > > > + else > > > + return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > > > } > > > > static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext) > > { > > if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext)) > > return true; > > > > return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > > } > > > > This is the code as things stand. If alternatives are disabled, the if > > statement becomes if (0 && foo) which will lead to the function call > > getting constant folded away and all you end up with is the call to > > __riscv_isa_extension_available(). Unless I am missing something, I don't > > think this patch has any affect? > > Yeah I fumbled this one it appears. I got thrown off by the nested > IS_ENABLED(CONFIG_RISCV_ALTERNATIVE). This patch eliminates the need for > this and maybe can avoid avoid confusion in the future. I think it just creates unneeded functions and can/should be dropped.
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index b5f4eedcfa86..db2ab037843a 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -90,22 +90,13 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned i __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext) static __always_inline bool -riscv_has_extension_likely(const unsigned long ext) +__riscv_has_extension_likely_alternatives(const unsigned long ext) { - compiletime_assert(ext < RISCV_ISA_EXT_MAX, - "ext must be < RISCV_ISA_EXT_MAX"); - - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { - asm goto( - ALTERNATIVE("j %l[l_no]", "nop", 0, %[ext], 1) - : - : [ext] "i" (ext) - : - : l_no); - } else { - if (!__riscv_isa_extension_available(NULL, ext)) - goto l_no; - } + asm goto(ALTERNATIVE("j %l[l_no]", "nop", 0, %[ext], 1) + : + : [ext] "i" (ext) + : + : l_no); return true; l_no: @@ -113,42 +104,63 @@ riscv_has_extension_likely(const unsigned long ext) } static __always_inline bool -riscv_has_extension_unlikely(const unsigned long ext) +__riscv_has_extension_unlikely_alternatives(const unsigned long ext) { - compiletime_assert(ext < RISCV_ISA_EXT_MAX, - "ext must be < RISCV_ISA_EXT_MAX"); - - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { - asm goto( - ALTERNATIVE("nop", "j %l[l_yes]", 0, %[ext], 1) - : - : [ext] "i" (ext) - : - : l_yes); - } else { - if (__riscv_isa_extension_available(NULL, ext)) - goto l_yes; - } + asm goto(ALTERNATIVE("nop", "j %l[l_yes]", 0, %[ext], 1) + : + : [ext] "i" (ext) + : + : l_yes); return false; l_yes: return true; } +static __always_inline bool +riscv_has_extension_likely(const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_EXT_MAX, + "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_likely_alternatives(ext); + else + return __riscv_isa_extension_available(NULL, ext); +} + +static __always_inline bool +riscv_has_extension_unlikely(const unsigned long ext) +{ + compiletime_assert(ext < RISCV_ISA_EXT_MAX, + "ext must be < RISCV_ISA_EXT_MAX"); + + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) + return __riscv_has_extension_unlikely_alternatives(ext); + else + return __riscv_isa_extension_available(NULL, ext); +} + static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext) { - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext)) - return true; + compiletime_assert(ext < RISCV_ISA_EXT_MAX, + "ext must be < RISCV_ISA_EXT_MAX"); - return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && __riscv_has_extension_likely_alternatives(ext)) + return true; + else + return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); } static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext) { - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext)) - return true; + compiletime_assert(ext < RISCV_ISA_EXT_MAX, + "ext must be < RISCV_ISA_EXT_MAX"); - return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && __riscv_has_extension_unlikely_alternatives(ext)) + return true; + else + return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); } #endif
When alternatives are disabled, riscv_cpu_isa_extension_(un)likely() checks if the current cpu supports the selected extension if not all cpus support the extension. It is sufficient to only check if the current cpu supports the extension. The alternatives code to handle if all cpus support an extension is factored out into a new function to support this. Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> --- arch/riscv/include/asm/cpufeature.h | 84 +++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 36 deletions(-)