On Wed, Dec 12, 2018 at 06:55:14PM -0600, Peter Bergner wrote:
> On 12/12/18 2:52 PM, Segher Boessenkool wrote:
> >> +/* This is a fairly new feature bit, so handle it not being defined. */
> >> +#ifndef PPC_FEATURE2_HTM_NO_SUSPEND
> >> +# define PPC_FEATURE2_HTM_NO_SUSPEND 0
> >> +#endif
> >
> > Doing it this way can be pretty surprising for users not aware you defined
> > it to 0. Since there are no other users yet it's no big deal.
>
> Would you instead prefer something along the lines of the following?
Yeah that looks a lot better :-)
> static inline bool
> htm_available (void)
> {
> - return (getauxval (AT_HWCAP2) & PPC_FEATURE2_HAS_HTM) ? true : false;
> +#ifdef __BUILTIN_CPU_SUPPORTS__
> + if (__builtin_cpu_supports ("htm-no-suspend")
> + || __builtin_cpu_supports ("htm"))
> + return true;
> +#else
> +#ifdef PPC_FEATURE2_HTM_NO_SUSPEND
> + unsigned long htm_flags = PPC_FEATURE2_HAS_HTM |
> PPC_FEATURE2_HTM_NO_SUSPEND;
> +#else
> + unsigned long htm_flags = PPC_FEATURE2_HAS_HTM;
> +#endif
> + if (getauxval (AT_HWCAP2) & htm_flags)
> + return true;
> +#endif
> + return false;
> }
Or like
unsigned long htm_flags = PPC_FEATURE2_HAS_HTM
#ifdef PPC_FEATURE2_HTM_NO_SUSPEND
| PPC_FEATURE2_HTM_NO_SUSPEND
#endif
| 0;
Okay for trunk with either style. Thanks!
Segher
