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 <clg@kaod.org>
---
Compile tested with --cross-prefix=x86_64-w64-mingw32-. When I have
some more time, I might try runtime on windows also.
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))
#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)
#else
# error Unknown sizeof long
#endif
--
2.17.2
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 <clg@kaod.org> > --- > > 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. > 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. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
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 <clg@kaod.org> >> --- >> >> 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.
On Mon, 17 Dec 2018 at 05:01, David Gibson <david@gibson.dropbear.id.au> wrote: > > On Sat, Dec 15, 2018 at 07:42:22PM +0100, Cédric Le Goater wrote: > > -#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. This is true, but I think "UL" is worth changing, as it's generally not what you want, being a type that might be 32 or 64 bit. Sensible suffix choices are "U", "ULL", or no suffix IMHO. thanks -- PMM
© 2016 - 2024 Red Hat, Inc.