concurrencykit / ck

Concurrency primitives, safe memory reclamation mechanisms and non-blocking (including lock-free) data structures designed to aid in the research, design and implementation of high performance concurrent systems developed in C99+.
http://concurrencykit.org/
Other
2.38k stars 313 forks source link

AArch64: missing clobbers in ck_pr_cas_64_2() and ck_pr_cas_64_2_value() #85

Closed akopytov closed 7 years ago

akopytov commented 7 years ago

I'm looking at ck_pr_cas_64_2() and ck_pr_cas_64_2_value() implementations in include/gcc/aarch64/ck_pr.h. I think the embedded asm statements in those functions are missing "memory" and "cc" clobbers. Other CAS implementation in the same file have them. Also, implementations of the same functions for x86_64 have those clobbers.

For example, this is from include/gcc/aarch64/ck_pr.h:

CK_CC_INLINE static bool
ck_pr_cas_64_2(uint64_t target[2], uint64_t compare[2], uint64_t set[2])
{
    uint64_t tmp1, tmp2;
    __asm__ __volatile__("1:"
                 "ldxp %0, %1, [%2];"
                 "eor %0, %0, %3;"
                 "eor %1, %1, %4;"
                 "orr %1, %0, %1;"
                 "mov %w0, #0;"
                 "cbnz %1, 2f;"
                 "stxp %w0, %5, %6, [%2];"
                 "cbnz %w0, 1b;"
                 "mov %w0, #1;"
                 "2:"
                 : "=&r" (tmp1), "=&r" (tmp2) 
                 : "r" (target), "r" (compare[0]), "r" (compare[1]), "r" (set[0]), "r" (set[1]));

    return (tmp1);
}

And this is from include/gcc/x86_64/ck_pr.h:

CK_CC_INLINE static bool
ck_pr_cas_64_2(uint64_t target[2], uint64_t compare[2], uint64_t set[2])
{
    bool z;

    __asm__ __volatile__("movq 0(%4), %%rax;"
                 "movq 8(%4), %%rdx;"
                 CK_PR_LOCK_PREFIX "cmpxchg16b %0; setz %1"
                : "+m" (*target),
                  "=q" (z)
                : "b"  (set[0]),
                  "c"  (set[1]),
                  "q"  (compare)
                : "memory", "cc", "%rax", "%rdx");
    return z;
}

Shouldn't AArch64 implementations also clobber "memory" and "cc"?

cognet commented 7 years ago

Hi Alexey,

It seems you are right, I'm going to fix that.

Thanks a lot !