0

Does anyone knows how can this function be changed to deal with 64bits?

{
  unsigned int prev;

  __asm__ __volatile__ (
          " lock; cmpxchgl %1,%2; "
          : "=a"(prev)
          : "q"(new_value), "m"(*(int *)ptr), "0"(old_value)
          : "memory");

  return prev;
}

Using unsigned long prev; and cmpxchgq instead of cmpxchgl as kindly suggested by Brett Hale results in these errors:

include/cs.h: Assembler messages:
include/cs.h:26: Error: incorrect register `%esi' used with `q'    suffix
include/cs.h:26: Error: incorrect register `%esi' used with `q' suffix
include/cs.h:26: Error: incorrect register `%esi' used with `q' suffix
include/cs.h:26: Error: incorrect register `%r13d' used with `q' suffix
error: command 'gcc' failed with exit status 1

I think I found the reason why Brett's suggestion did not work for me. I had to the change the variable types in the function input from int to long. For completeness I am adding it here:

#ifndef __cs__include
#define __cs__include

static inline unsigned int CS(volatile void *ptr,
                              unsigned long old_value, /* was int */
                              unsigned long new_value) /* was int too */
{
  unsigned long prev; /* result */
  volatile unsigned long *vptr = (volatile unsigned long *) ptr;

  __asm__ __volatile__ (

          " lock; cmpxchgq %2, %1; "
          : "=a" (prev), "+m" (*vptr)
          : "r" (new_value), "0" (old_value)
          : "memory");

  return prev;
}

The code compiles without errors (though there are many warnings). However, the program does unfortunately still not work on 64bit.

Hevok
  • 13
  • 1
  • 4
  • See http://stackoverflow.com/questions/833122/cmpxchg-example-for-64-bit-integer – Michael Jun 13 '13 at 16:28
  • 1
    This inline asm is gcc-specific, so you might as well use gcc atomic builtins instead. – Jester Jun 13 '13 at 16:55
  • @Jester - that seems strangely dismissive from someone who is curious about assembly language. – Brett Hale Jun 13 '13 at 17:10
  • @BrettHale Use the right tool for the job. I don't see any advantage of inline asm for this purpose. – Jester Jun 13 '13 at 17:13
  • Thanks Michael. I looked into `cmpxchg example for 64 bit integer` as it is related, but found it not helpful. – Hevok Jun 13 '13 at 17:52
  • @Jester: How would an implementation of this with the use of gcc atomic builtins look like? – Hevok Jun 13 '13 at 17:53

2 Answers2

2

The builtin version (with __sync style) looks like this:

#include <stdint.h>
#include <stdio.h>

uint64_t cas(uint64_t* ptr, uint64_t old_value, uint64_t new_value)
{
    return __sync_val_compare_and_swap(ptr, old_value, new_value);
}

int main()
{
    uint64_t foo = 42;
    uint64_t old = cas(&foo, 42, 1);
    printf("foo=%llu old=%llu\n", (unsigned long long)foo, (unsigned long long)old);
    return 0;
}

The beauty of this is that it works on many architectures. On x86 it uses cmpxchg8b in 32 bit mode and cmpxchgq in 64 bit mode.

Your question wasn't quite clear, maybe you intended to keep 32 bit operation while compiling for 64 bit mode. In that case, use uint32_t instead of uint64_t.

Jester
  • 56,577
  • 4
  • 81
  • 125
0

First, since we're presumably dealing with an LP64 data model, use: unsigned long prev; for a 64-bit quantity. Then replace cmpxchgl with cmpxchgq (64-bit instruction).

  • Operand %1 has the "q" constraint, which restricted the choice to %eax, %ebx, %ecx, or %edx with IA32. That restriction doesn't apply for x86-64. According to this, we should be able to leave it as "q", but it's better described with "r".

  • Operand %2 is a memory input, which should also be promoted to unsigned long. It should probably be marked as a volatile fetch also. This prevents the compiler deciding when it would prefer to fetch / update memory beforehand.

  • Operand %3 just means that %rax is an input as well as an output - it shouldn't need changing.


{
  unsigned long prev; /* result */
  volatile unsigned long *vptr = (volatile unsigned long *) ptr;

  __asm__ __volatile__ (

          " lock; cmpxchgq %1,%2; "
          : "=a"(prev)
          : "r"(new_value), "m"(*vptr), "0"(old_value)
          : "memory");

  return prev;
}

However, the use of "m" in the input constraints isn't technically correct, since it can be updated by the cmpxchgq instruction. There was a long-standing bug in the gcc documentation, now corrected, that stated "+m" (memory as both an input and output) wasn't allowed. The more correct expression is:

  __asm__ __volatile__ (

          " lock; cmpxchgq %2, %1; " /* notice reversed operand order! */
          : "=a" (prev), "+m" (*vptr)
          : "r" (new_value), "0" (old_value)
          : "memory");
Brett Hale
  • 21,653
  • 2
  • 61
  • 90
  • Dear Brett, Thank you very much. I tried both suggestions, but it raises the incorrect register errors as detailed in the question above. – Hevok Jun 13 '13 at 17:36
  • @Hevok - I don't know how you are compiling, or what system you're on, but are you compiling for 64-bit code? e.g. `gcc -m64`? – Brett Hale Jun 13 '13 at 17:53
  • Your explanation is very helpful for understanding. Might there be something missing to make it work? – Hevok Jun 13 '13 at 17:58
  • Yes, this is correct I am compiling on a Unix system that is of 64bit architecture. However, I did not explicitly added the `-m64` flag. I tried it just out and it had no effect. – Hevok Jun 13 '13 at 18:00
  • So this solution appears to be right. I am not yet 100% certain as the program still does not work. Thank you so far. – Hevok Jun 13 '13 at 19:59