[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target/ppc: fix compilation breakage on windows
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] [PATCH] target/ppc: fix compilation breakage on windows |
Date: |
Mon, 17 Dec 2018 08:58:45 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 |
On 12/17/18 5:50 AM, David Gibson wrote:
> On Sat, Dec 15, 2018 at 07:42:22PM +0100, Cédric Le Goater wrote:
>> Fix the PPC_BIT definitions to use ULL instead in UL and replace
>> __builtin_ffssl() by the equivalent ctz routines.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>
>> Compile tested with --cross-prefix=x86_64-w64-mingw32-. When I have
>> some more time, I might try runtime on windows also.
>
> Which version of the mingw compiler do you have? As I've noted
> elsewhere, I haven't been able to reproduce the problem with the mingw
> in Fedora 29.
I have :
mingw64-gcc-7.3.0-1.fc28.x86_64
>> The PPC compile failures have been there for a while (pre 2.12) and
>> the MASK_TO_LSH macro was not used until the XIVE definitions were
>> introduced.
>>
>> I let you guys decide on how you want to proceed, but I think it is
>> safe to merge this patch as a prereq of the pull request.
>>
>> Thanks,
>>
>> C.
>>
>> target/ppc/cpu.h | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index ab68abe8a23c..a3d3e91eb4ce 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -70,18 +70,18 @@
>> #define PPC_ELF_MACHINE EM_PPC
>> #endif
>>
>> -#define PPC_BIT(bit) (0x8000000000000000UL >> (bit))
>> -#define PPC_BIT32(bit) (0x80000000UL >> (bit))
>> -#define PPC_BIT8(bit) (0x80UL >> (bit))
>> +#define PPC_BIT(bit) (0x8000000000000000ULL >> (bit))
>> +#define PPC_BIT32(bit) (0x80000000ULL >> (bit))
>> +#define PPC_BIT8(bit) (0x80ULL >> (bit))
>
> We shouldn't need the ULL for the 32-bit and (especially) 8-bit
> versions.
>
>> #define PPC_BITMASK(bs, be) ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
>> #define PPC_BITMASK32(bs, be) ((PPC_BIT32(bs) - PPC_BIT32(be)) | \
>> PPC_BIT32(bs))
>> #define PPC_BITMASK8(bs, be) ((PPC_BIT8(bs) - PPC_BIT8(be)) |
>> PPC_BIT8(bs))
>>
>> #if HOST_LONG_BITS == 32
>> -# define MASK_TO_LSH(m) (__builtin_ffsll(m) - 1)
>> +# define MASK_TO_LSH(m) ctz32(m)
>> #elif HOST_LONG_BITS == 64
>> -# define MASK_TO_LSH(m) (__builtin_ffsl(m) - 1)
>> +# define MASK_TO_LSH(m) ctz64(m)
>
> Having another look at this, this seems bogus: we're defining an
> interface that's only correct on a *host long*. But when we use it
> we're almost certain to be working on guest values with a fixed size,
> so using this wrapper just adds confusion.
>
> I'm not really seeing any reason to use MASK_TO_LSH() rather than
> using ctz32() and ctz64() explicitly.
Does that mean you think we should define GETFIELD32/64 and
SETFIELD32/64 macros ?
or should we use the extract32/64 and deposit32/64 helpers instead ?
That's a bigger change but may be a better one.
Also, in the definitions above, only PPC_BIT() needs to use a ULL.
C.