[v2] RISC-V: Use builtins for ffs and ffsll while supported extension available

Message ID 20240619112611.2555-1-jz531210@gmail.com (mailing list archive)
State New
Headers
Series [v2] RISC-V: Use builtins for ffs and ffsll while supported extension available |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed

Commit Message

Julian Zhu June 19, 2024, 11:26 a.m. UTC
  Hardware `ctz`/`ctzw` instructions are available in the RISC-V Zbb extension, and the `ctz` instruction `th.ff1` is available in the RISC-V XTheadBb extension. We can  get more simplified code compared to the generic implement of `ffs`/`ffsll`.

Signed-off-by: Julian Zhu <jz531210@gmail.com>
---
 sysdeps/riscv/math-use-builtins-ffs.h | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 sysdeps/riscv/math-use-builtins-ffs.h
  

Comments

Adhemerval Zanella July 1, 2024, 12:15 p.m. UTC | #1
On 19/06/24 08:26, Julian Zhu wrote:
> Hardware `ctz`/`ctzw` instructions are available in the RISC-V Zbb extension, and the `ctz` instruction `th.ff1` is available in the RISC-V XTheadBb extension. We can  get more simplified code compared to the generic implement of `ffs`/`ffsll`.
> 
> Signed-off-by: Julian Zhu <jz531210@gmail.com>

Does it work for all support gcc compilers on riscv, or do we need an specific
version to get the correct lowering?

> ---
>  sysdeps/riscv/math-use-builtins-ffs.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>  create mode 100644 sysdeps/riscv/math-use-builtins-ffs.h
> 
> diff --git a/sysdeps/riscv/math-use-builtins-ffs.h b/sysdeps/riscv/math-use-builtins-ffs.h
> new file mode 100644
> index 0000000000..a803be5cb3
> --- /dev/null
> +++ b/sysdeps/riscv/math-use-builtins-ffs.h
> @@ -0,0 +1,7 @@
> +#if defined __riscv_zbb
> +#define USE_FFS_BUILTIN   1
> +#define USE_FFSLL_BUILTIN 1
> +#elif __riscv_xtheadbb
> +#define USE_FFS_BUILTIN   0
> +#define USE_FFSLL_BUILTIN 1
> +#endif
  
Xi Ruoyao July 1, 2024, 12:20 p.m. UTC | #2
On Mon, 2024-07-01 at 09:15 -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 19/06/24 08:26, Julian Zhu wrote:
> > Hardware `ctz`/`ctzw` instructions are available in the RISC-V Zbb extension, and the `ctz` instruction `th.ff1` is available in the RISC-V XTheadBb extension. We can  get more simplified code compared to the generic implement of `ffs`/`ffsll`.
> > 
> > Signed-off-by: Julian Zhu <jz531210@gmail.com>
> 
> Does it work for all support gcc compilers on riscv, or do we need an specific
> version to get the correct lowering?

At least for zbb you need to check GCC >= 12 or you'll make an infinite
recursion: https://godbolt.org/z/re3hbEa15

> > ---
> >  sysdeps/riscv/math-use-builtins-ffs.h | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >  create mode 100644 sysdeps/riscv/math-use-builtins-ffs.h
> > 
> > diff --git a/sysdeps/riscv/math-use-builtins-ffs.h b/sysdeps/riscv/math-use-builtins-ffs.h
> > new file mode 100644
> > index 0000000000..a803be5cb3
> > --- /dev/null
> > +++ b/sysdeps/riscv/math-use-builtins-ffs.h
> > @@ -0,0 +1,7 @@
> > +#if defined __riscv_zbb
> > +#define USE_FFS_BUILTIN   1
> > +#define USE_FFSLL_BUILTIN 1
> > +#elif __riscv_xtheadbb
> > +#define USE_FFS_BUILTIN   0
> > +#define USE_FFSLL_BUILTIN 1
> > +#endif
  
Julian Zhu July 2, 2024, 1:39 p.m. UTC | #3
在 2024/7/1 20:20, Xi Ruoyao 写道:
> On Mon, 2024-07-01 at 09:15 -0300, Adhemerval Zanella Netto wrote:
>>
>> On 19/06/24 08:26, Julian Zhu wrote:
>>> Hardware `ctz`/`ctzw` instructions are available in the RISC-V Zbb extension, and the `ctz` instruction `th.ff1` is available in the RISC-V XTheadBb extension. We can  get more simplified code compared to the generic implement of `ffs`/`ffsll`.
>>>
>>> Signed-off-by: Julian Zhu <jz531210@gmail.com>
>> Does it work for all support gcc compilers on riscv, or do we need an specific
>> version to get the correct lowering?
> At least for zbb you need to check GCC >= 12 or you'll make an infinite
> recursion: https://godbolt.org/z/re3hbEa15
>
>>> ---
>>>   sysdeps/riscv/math-use-builtins-ffs.h | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>   create mode 100644 sysdeps/riscv/math-use-builtins-ffs.h
>>>
>>> diff --git a/sysdeps/riscv/math-use-builtins-ffs.h b/sysdeps/riscv/math-use-builtins-ffs.h
>>> new file mode 100644
>>> index 0000000000..a803be5cb3
>>> --- /dev/null
>>> +++ b/sysdeps/riscv/math-use-builtins-ffs.h
>>> @@ -0,0 +1,7 @@
>>> +#if defined __riscv_zbb
>>> +#define USE_FFS_BUILTIN   1
>>> +#define USE_FFSLL_BUILTIN 1
>>> +#elif __riscv_xtheadbb
>>> +#define USE_FFS_BUILTIN   0
>>> +#define USE_FFSLL_BUILTIN 1
>>> +#endif
>>>

Thank you for your feedback. I will send a new series of patches to the 
mailing list later.
  

Patch

diff --git a/sysdeps/riscv/math-use-builtins-ffs.h b/sysdeps/riscv/math-use-builtins-ffs.h
new file mode 100644
index 0000000000..a803be5cb3
--- /dev/null
+++ b/sysdeps/riscv/math-use-builtins-ffs.h
@@ -0,0 +1,7 @@ 
+#if defined __riscv_zbb
+#define USE_FFS_BUILTIN   1
+#define USE_FFSLL_BUILTIN 1
+#elif __riscv_xtheadbb
+#define USE_FFS_BUILTIN   0
+#define USE_FFSLL_BUILTIN 1
+#endif