* [PATCH, rs6000] Allow libitm to use HTM on newer hw and kernels
@ 2018-12-12 19:47 Peter Bergner
2018-12-12 20:52 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Peter Bergner @ 2018-12-12 19:47 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches
Libitm on POWER hardware looks for the "htm" bit in AT_HWCAP2 to determine
whether it can use HTM when executing code within __transaction_atomic
code blocks. However, on newer hardware and kernels, the "htm" bit is no
longer set and instead the "htm-no-suspend" bit is set, so we currently
don't use HTM on new hw and kernels. The following patch adds support
for htm-no-suspend to libitm. I have also added code to use the
__builtin_cpu_supports() builtin if it is available, since that is
much faster than using the getauxval libc call.
This passed bootstrap and regtesting with no errors and someone within
IBM how had a POWER9 box with a newish kernel how ran into the problem
confirmed it works for his test case.
Ok for mainline? Should be backport this?
Peter
* config/powerpc/target.h (PPC_FEATURE2_HTM_NO_SUSPEND): Conditionally
define.
(htm_available): Add support for PPC_FEATURE2_HTM_NO_SUSPEND.
Use __builtin_cpu_supports if available.
Index: libitm/config/powerpc/target.h
===================================================================
--- libitm/config/powerpc/target.h (revision 267062)
+++ libitm/config/powerpc/target.h (working copy)
@@ -26,6 +26,11 @@
#include <sys/auxv.h>
#endif
+/* 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
+
namespace GTM HIDDEN {
typedef int v128 __attribute__((vector_size(16), may_alias, aligned(16)));
@@ -81,7 +86,16 @@ cpu_relax (void)
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
+ if (getauxval (AT_HWCAP2)
+ & (PPC_FEATURE2_HAS_HTM | PPC_FEATURE2_HTM_NO_SUSPEND))
+ return true;
+#endif
+ return false;
}
static inline uint32_t
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH, rs6000] Allow libitm to use HTM on newer hw and kernels
2018-12-12 19:47 [PATCH, rs6000] Allow libitm to use HTM on newer hw and kernels Peter Bergner
@ 2018-12-12 20:52 ` Segher Boessenkool
2018-12-13 0:55 ` Peter Bergner
0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2018-12-12 20:52 UTC (permalink / raw)
To: Peter Bergner; +Cc: GCC Patches
On Wed, Dec 12, 2018 at 01:47:02PM -0600, Peter Bergner wrote:
> Libitm on POWER hardware looks for the "htm" bit in AT_HWCAP2 to determine
> whether it can use HTM when executing code within __transaction_atomic
> code blocks. However, on newer hardware and kernels, the "htm" bit is no
> longer set and instead the "htm-no-suspend" bit is set, so we currently
> don't use HTM on new hw and kernels. The following patch adds support
> for htm-no-suspend to libitm. I have also added code to use the
> __builtin_cpu_supports() builtin if it is available, since that is
> much faster than using the getauxval libc call.
>
> This passed bootstrap and regtesting with no errors and someone within
> IBM how had a POWER9 box with a newish kernel how ran into the problem
> confirmed it works for his test case.
>
> Ok for mainline? Should be backport this?
Okay for trunk (but see comment below). I think it should be backported
yes, to 8 at least, probably 7. Okay for those too.
> +/* 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.
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, rs6000] Allow libitm to use HTM on newer hw and kernels
2018-12-12 20:52 ` Segher Boessenkool
@ 2018-12-13 0:55 ` Peter Bergner
2018-12-13 11:12 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Peter Bergner @ 2018-12-13 0:55 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches
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?
Peter
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;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH, rs6000] Allow libitm to use HTM on newer hw and kernels
2018-12-13 0:55 ` Peter Bergner
@ 2018-12-13 11:12 ` Segher Boessenkool
2018-12-13 16:42 ` Peter Bergner
0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2018-12-13 11:12 UTC (permalink / raw)
To: Peter Bergner; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH, rs6000] Allow libitm to use HTM on newer hw and kernels
2018-12-13 11:12 ` Segher Boessenkool
@ 2018-12-13 16:42 ` Peter Bergner
2018-12-13 18:11 ` Peter Bergner
0 siblings, 1 reply; 6+ messages in thread
From: Peter Bergner @ 2018-12-13 16:42 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches
On 12/13/18 2:41 AM, Segher Boessenkool wrote:
> 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!
Ok, I'll go with this and commit once I've done another round of
testing. I'm assuming your backport approval still stands for this
code too.
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, rs6000] Allow libitm to use HTM on newer hw and kernels
2018-12-13 16:42 ` Peter Bergner
@ 2018-12-13 18:11 ` Peter Bergner
0 siblings, 0 replies; 6+ messages in thread
From: Peter Bergner @ 2018-12-13 18:11 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches
On 12/13/18 10:42 AM, Peter Bergner wrote:
> On 12/13/18 2:41 AM, Segher Boessenkool wrote:
>> 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!
>
> Ok, I'll go with this and commit once I've done another round of
> testing. I'm assuming your backport approval still stands for this
> code too.
Testing was clean everywhere. Committed everywhere. Thanks!
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-13 18:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-12 19:47 [PATCH, rs6000] Allow libitm to use HTM on newer hw and kernels Peter Bergner
2018-12-12 20:52 ` Segher Boessenkool
2018-12-13 0:55 ` Peter Bergner
2018-12-13 11:12 ` Segher Boessenkool
2018-12-13 16:42 ` Peter Bergner
2018-12-13 18:11 ` Peter Bergner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).