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

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 11/1/24 10:48, Peter Maydell wrote:
> 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 ?
> 

Adding Tomasz (author of this change) and Alistair, who might have an 
opinion on this.

> thanks
> -- PMM