3

What's wrong with this code? Compiler returns ambiguous operand size for dec in unlock function. But, there is qword keyword (size specifier).

class spinlock
{
    qword locked;
public:
    spinlock() : locked(-1)
    { }
    void lock()     __attribute__((noinline));
    void unlock()   __attribute__((noinline));
};

void spinlock::lock()
{
    asm(
    ".intel_syntax noprefix;"
    "xor rcx, rcx;"
    "loop:;"
    "lock xchg qword [%0], rcx;"
    "test rcx, rcx;"
    "jz loop;"
    ".att_syntax noprefix;"
    : 
    : "r"(&locked)
    : "rcx"
    );
}

void spinlock::unlock()
{
    asm(
    ".intel_syntax noprefix;"
    "lock dec qword [%0];"
    ".att_syntax noprefix;"
    :
    : "r"(&locked)
    :
    );
}
Cœur
  • 37,241
  • 25
  • 195
  • 267
dev1223
  • 1,148
  • 13
  • 28
  • 1
    not sure what the `%0`is (something g++?), but at least with old MASM and TASM the proper syntax was like `QWORD PTR` *operand*. – Cheers and hth. - Alf Apr 09 '14 at 17:45
  • 1
    you really should give more information. see this C++ FAQ item about **[how to post a question about code that does not work](http://www.parashift.com/c++-faq/posting-code.html)**. even if it's for another more technical forum, not SO. note especially points **5**, **6** and **7**. – Cheers and hth. - Alf Apr 09 '14 at 17:47
  • you are right ... I forgotten ptr keyword ... – dev1223 Apr 09 '14 at 18:01
  • For future readers, **this code is unsafe and slow.** A pointer operand does not imply that the pointed-to memory is also an operand, and spinning on `xchg` instead of spinning read-only with a `pause` is generally worse. Also, there's no reason to make these `noinline`: a `"memory"` clobber would make it a full barrier against compile-time reordering. Unlock doesn't need a `lock` prefix; a release-store of a constant is sufficient. See [Locks around memory manipulation via inline assembly](https://stackoverflow.com/a/37246263) for a better example. – Peter Cordes Nov 14 '18 at 15:12

0 Answers0