| Message ID | 20120411105101.GA16117@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 11, 2012 at 12:51 PM, Jakub Jelinek <jakub@redhat.com> wrote: > What is TARGET_HLE good for? I thought the point of HLE prefixes > is that they are silently ignored on older CPUs. So, HLE should be > always enabled IMHO. If you don't use __ATOMIC_HLE_* bits in __atomic_* > in your source, it won't be emitted, if you use them, you are supposedly > intending to compile code that will use normal locking on older CPUs > and HLE TM on new CPUs. I think that we should keep -mhle, since it controls if we want HLE prefixes or not, saving a byte per lock prefix if we know that binary won't run on HLE enabled processor. You will also need to check assembler support for new prefixes and emit ASM_BYTE "0xXX" if not supported. Please see how HAVE_AS_IX86_SAHF is handled. +{ + static char buf[128], hle[16]=""; + if (INTVAL (operands[4]) & IX86_HLE_ACQUIRE) + snprintf (hle, sizeof (hle), "xacquire "); + else if (INTVAL (operands[4]) & IX86_HLE_RELEASE) + snprintf (hle, sizeof (hle), "release "); + Ouch... const char *hle; if (INTVAL (...) hle = "xacquire "; else if (INTVAL (...) hle = "xrelease "; else hle = ""; I assume that all this will be moved to a helper function that will also handle HAVE_AS_IX86_HLE. Uros.
--- /dev/null +++ b/gcc/testsuite/gcc.target/i386/hle-cmpxchg-rel-1.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-mhle -dp" } */ +/* { dg-final { scan-assembler "lock release cmpxchg" } } */ Isn't the prefix called xrelease? At least in my binutils version it is... And, why the -dp switch? + +int
Hi! On Wed, Apr 11, 2012 at 02:35:38PM +0400, Kirill Yukhin wrote: What is TARGET_HLE good for? I thought the point of HLE prefixes is that they are silently ignored on older CPUs. So, HLE should be always enabled IMHO. If you don't use __ATOMIC_HLE_* bits in __atomic_* in your source, it won't be emitted, if you use them, you are supposedly intending to compile code that will use normal locking on older CPUs and HLE TM on new CPUs. + if (isa_flag & OPTION_MASK_ISA_HLE) { + char buf[64]; + + sprintf (buf, "__ATOMIC_HLE_ACQUIRE=%d", IX86_HLE_ACQUIRE); + def_or_undef (parse_in, buf); + + sprintf (buf, "__ATOMIC_HLE_RELEASE=%d", IX86_HLE_RELEASE); + def_or_undef (parse_in, buf); So IMHO the above two macros should be defined always. + def_or_undef (parse_in, "__HLE__"); And I don't see a point for this macro (nor the -m*=native stuff in the patch). @@ -2344,6 +2345,12 @@ extern void debug_dispatch_window (int); #define TARGET_RECIP_VEC_DIV ((recip_mask & RECIP_MASK_VEC_DIV) != 0) #define TARGET_RECIP_VEC_SQRT ((recip_mask & RECIP_MASK_VEC_SQRT) != 0) +#define IX86_HLE_ACQUIRE (1 << (sizeof(MEMMODEL_LAST) * 8 - \ + __builtin_clz (MEMMODEL_LAST))) + +#define IX86_HLE_RELEASE (1 << (sizeof(MEMMODEL_LAST) * 8 - \ + __builtin_clz (MEMMODEL_LAST) - 1)) I don't think you can use __builtin_clz in GCC source, at least not conditionally on the host compiler. If you use clz_hwi instead, it will DTRT even for compilers that don't support __builtin_clz. +hle_cmpxchg (int *p, int oldv, int newv) +{ + return __atomic_compare_exchange_n (p, &oldv, newv, 0, __ATOMIC_RELEASE | __ATOMIC_HLE_RELEASE, __ATOMIC_ACQUIRE); +} Jakub