| Message ID | 1480937130-24561-3-git-send-email-nikunj@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/05/2016 03:25 AM, Nikunj A Dadhania wrote: > +#if defined(CONFIG_INT128) > +FUNC_MASK(mask_u128, Int128, 128, Int128, ~((__uint128_t)0)); > +#else > +static inline Int128 mask_u128(int start, int end) > +{ > + Int128 r = {0}; > + if (start > 63) { > + r.hi = 0; > + r.lo = mask_u64(start - 64, end - 64); > + } else if (end < 64) { > + r.hi = mask_u64(start, end); > + r.lo = 0; > + } else { > + r.hi = mask_u64(start, 63); > + r.lo = mask_u64(0, end - 64); > + } > + return r; > +} > #endif First, I would really really like you to stop adding *any* ifdefs on CONFIG_INT128. All that's going to do is make sure that there's code that is almost never tested, since x86_64 (and other 64-bit hosts) does support int128. Second, you're not using the Int128 interface correctly. Better would be static inline Int128 mask_u128(int start, int end) { uint64_t hi, lo; if (start > 63) { hi = 0; lo = mask_u64(start - 64, end - 64); } else if (end < 64) { hi = mask_u64(start, end); lo = 0; } else { hi = mask_u64(start, 63); lo = mask_u64(0, end - 64); } return make_int128(lo, hi); } r~
Richard Henderson <rth@twiddle.net> writes: > On 12/05/2016 03:25 AM, Nikunj A Dadhania wrote: >> +#if defined(CONFIG_INT128) >> +FUNC_MASK(mask_u128, Int128, 128, Int128, ~((__uint128_t)0)); >> +#else >> +static inline Int128 mask_u128(int start, int end) >> +{ >> + Int128 r = {0}; >> + if (start > 63) { >> + r.hi = 0; >> + r.lo = mask_u64(start - 64, end - 64); >> + } else if (end < 64) { >> + r.hi = mask_u64(start, end); >> + r.lo = 0; >> + } else { >> + r.hi = mask_u64(start, 63); >> + r.lo = mask_u64(0, end - 64); >> + } >> + return r; >> +} >> #endif > > First, I would really really like you to stop adding *any* ifdefs on > CONFIG_INT128. All that's going to do is make sure that there's code that is > almost never tested, since x86_64 (and other 64-bit hosts) does support int128. I did test both the cases above by flipping the switch of CONFIG_INT128. Initially was planning to do this in int128.h, but the bit numbering is different and wont be usable for other architecture. > Second, you're not using the Int128 interface correctly. Better would be > > static inline Int128 mask_u128(int start, int end) > { > uint64_t hi, lo; > if (start > 63) { > hi = 0; > lo = mask_u64(start - 64, end - 64); > } else if (end < 64) { > hi = mask_u64(start, end); > lo = 0; > } else { > hi = mask_u64(start, 63); > lo = mask_u64(0, end - 64); > } > return make_int128(lo, hi); > } Sure will use this. Regards Nikunj
diff --git a/target-ppc/internal.h b/target-ppc/internal.h index 66cde46..27d956f 100644 --- a/target-ppc/internal.h +++ b/target-ppc/internal.h @@ -18,9 +18,9 @@ #ifndef PPC_INTERNAL_H #define PPC_INTERNAL_H -#define FUNC_MASK(name, ret_type, size, max_val) \ -static inline ret_type name(uint##size##_t start, \ - uint##size##_t end) \ +#define FUNC_MASK(name, ret_type, size, in_type, max_val) \ +static inline ret_type name(in_type start, \ + in_type end) \ { \ ret_type ret, max_bit = size - 1; \ \ @@ -29,8 +29,8 @@ static inline ret_type name(uint##size##_t start, \ } else if (likely(end == max_bit)) { \ ret = max_val >> start; \ } else { \ - ret = (((uint##size##_t)(-1ULL)) >> (start)) ^ \ - (((uint##size##_t)(-1ULL) >> (end)) >> 1); \ + ret = (((in_type)(-1ULL)) >> (start)) ^ \ + (((in_type)(-1ULL) >> (end)) >> 1); \ if (unlikely(start > end)) { \ return ~ret; \ } \ @@ -40,12 +40,32 @@ static inline ret_type name(uint##size##_t start, \ } #if defined(TARGET_PPC64) -FUNC_MASK(MASK, target_ulong, 64, UINT64_MAX); +FUNC_MASK(MASK, target_ulong, 64, uint64_t, UINT64_MAX); #else -FUNC_MASK(MASK, target_ulong, 32, UINT32_MAX); +FUNC_MASK(MASK, target_ulong, 32, uint32_t, UINT32_MAX); +#endif +FUNC_MASK(mask_u32, uint32_t, 32, uint32_t, UINT32_MAX); +FUNC_MASK(mask_u64, uint64_t, 64, uint64_t, UINT64_MAX); + +#if defined(CONFIG_INT128) +FUNC_MASK(mask_u128, Int128, 128, Int128, ~((__uint128_t)0)); +#else +static inline Int128 mask_u128(int start, int end) +{ + Int128 r = {0}; + if (start > 63) { + r.hi = 0; + r.lo = mask_u64(start - 64, end - 64); + } else if (end < 64) { + r.hi = mask_u64(start, end); + r.lo = 0; + } else { + r.hi = mask_u64(start, 63); + r.lo = mask_u64(0, end - 64); + } + return r; +} #endif -FUNC_MASK(mask_u32, uint32_t, 32, UINT32_MAX); -FUNC_MASK(mask_u64, uint64_t, 64, UINT64_MAX); /*****************************************************************************/ /*** Instruction decoding ***/
Adjust FUNC_MASK define and add function to generate mask_u128 Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- target-ppc/internal.h | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-)