Re: [PATCH] hw/riscv: fix build error with clang

[Peter Maydell]

On Fri, 1 Nov 2024 at 17:36, Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 11/1/24 2:08 PM, Pierrick Bouvier wrote:
> > Introduced in 0c54ac, "hw/riscv: add RISC-V IOMMU base emulation"
> >
> > ../hw/riscv/riscv-iommu.c:187:17: error: redefinition of '_pext_u64'
> >
> >    187 | static uint64_t _pext_u64(uint64_t val, uint64_t ext)
> >
> >        |                 ^
> >
> > D:/a/_temp/msys64/clang64/lib/clang/18/include/bmi2intrin.h:217:1: note: 
> > previous definition is here
> >
> >    217 | _pext_u64(unsigned long long __X, unsigned long long __Y)
> >
> >        | ^
> >
> > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > ---
> >   hw/riscv/riscv-iommu.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> > index feb650549ac..f738570bac2 100644
> > --- a/hw/riscv/riscv-iommu.c
> > +++ b/hw/riscv/riscv-iommu.c
> > @@ -184,7 +184,7 @@ static void riscv_iommu_pri(RISCVIOMMUState *s,
> >   }
> >
> >   /* Portable implementation of pext_u64, bit-mask extraction. */
> > -static uint64_t _pext_u64(uint64_t val, uint64_t ext)
> > +static uint64_t pext_u64(uint64_t val, uint64_t ext)
>
> I suggest name it 'riscv_iommu_pext_u64' to be clear that this is a local 
> scope function,
> not to be mistaken with anything available in clang or any other compiler.

More generally, we should avoid using leading '_' in QEMU function
names; those are reserved for the system.

Also, what does this function do? The comment assumes that
the reader knows what a "pext_u64" function does, but if you
don't then it's fairly inscrutable bit-twiddling.
"bit-mask extraction" suggests maybe we should be using
the bitops.h extract functions instead ?

thanks
-- PMM