0

I have to implement set_bit function which should be a atomic operation. I found assembly code for this in linux source. (I'm using sparc) and want to change it to a function that can be used in C program.

    static void set_bit(unsigned int nr, unsigned int *addr)
    {
    //  *vec |= 1<<bit;  <== original non-atomic C code
    //set_bit:      /* %o0=nr, %o1=addr */   <== nr is in %o0, addr in %o1 by sparc rule
__asm__ __volatile__ (
    "srlx   %o0, 6, %g1"
    "mov    1, %o2"
    "sllx   %g1, 3, %g3"
    "and    %o0, 63, %g2"
    "sllx   %o2, %g2, %o2"
    "add    %o1, %g3, %o1"
"1: ldx [%o1], %g7"
    "or %g7, %o2, %g1"
    "casx   [%o1], %g7, %g1"
    "cmp    %g7, %g1"
    "bne,pn %xcc, 2f"
     "nop"
    "retl"
    "nop"
    : "=m"(addr) // output
    : "m"(nr) // input
    : );

Is this correct? Do I have list up all the clobberd registers at the last line?

I'm see error messages below..

../../../../../rtems-4.10.99-src/c/src/libchip/sdmmc/ald-sd-card.c:135:1: error: invalid 'asm': invalid operand output code
 __asm__ __volatile__ (
 ^
../../../../../rtems-4.10.99-src/c/src/libchip/sdmmc/ald-sd-card.c:135:1: error: invalid 'asm': invalid operand output code
../../../../../rtems-4.10.99-src/c/src/libchip/sdmmc/ald-sd-card.c:135:1: error: invalid 'asm': operand number out of range

      ^
Rob
  • 14,746
  • 28
  • 47
  • 65
Chan Kim
  • 5,177
  • 12
  • 57
  • 112
  • If you don't require assembly, just let the compiler take care of it and use `__atomic_or_fetch`. – Jester Sep 05 '15 at 12:01
  • you mean where to put it in? for example if I have code set_bit(nr, &events); ? you mean like __atomic_or_fetch *vec |= 1< – Chan Kim Sep 05 '15 at 12:02
  • You can put it there, or in the `set_bit` of course. No, you use it like `__atomic_or_fetch(addr, 1 << nr, __ATOMIC_SEQ_CST)`. – Jester Sep 05 '15 at 12:03
  • I'm seeing ../../../../../xxxx/ald-sd-card.c:134: more undefined references to `__atomic_fetch_or_4' follow. :( Thanks anyway :) – Chan Kim Sep 05 '15 at 12:13
  • Your compiler version may be too old then (you didn't say). You could try the legacy `__sync_or_and_fetch(addr, 1 << nr)` then. – Jester Sep 05 '15 at 12:26

2 Answers2

0

I think you should write only one instruction per line in the string (add \n\t at the end of each instructions).

    static void set_bit(unsigned int nr, unsigned int *addr)
    {
    //  *vec |= 1<<bit;  <== original non-atomic C code
    //set_bit:      /* %o0=nr, %o1=addr */   <== nr is in %o0, addr in %o1 by sparc rule
__asm__ __volatile__ (
    "srlx   %o0, 6, %g1\n\t"
    "mov    1, %o2\n\t"
    "sllx   %g1, 3, %g3\n\t"
    "and    %o0, 63, %g2\n\t"
    "sllx   %o2, %g2, %o2\n\t"
    "add    %o1, %g3, %o1\n\t"
"1: ldx [%o1], %g7\n\t"
    "or %g7, %o2, %g1\n\t"
    "casx   [%o1], %g7, %g1\n\t"
    "cmp    %g7, %g1\n\t"
    "bne,pn %xcc, 2f\n\t"
     "nop\n\t"
    "retl\n\t"
    "nop\n\t"
    : "=m"(addr) // output
    : "m"(nr) // input
    : );
    }
MikeCAT
  • 73,922
  • 11
  • 45
  • 70
  • I tried this but got : I avoid using bit operation and now it works fine. ../../../../../rtems-4.10.99-src/c/src/libchip/sdmmc/ald-sd-card.c:159:1: error: invalid 'asm': invalid operand output code __asm__ __volatile__ ( ^ ../../../../../rtems-4.10.99-src/c/src/libchip/sdmmc/ald-sd-card.c:159:1: error: invalid 'asm': invalid operand output code ../../../../../rtems-4.10.99-src/c/src/libchip/sdmmc/ald-sd-card.c:159:1: error: invalid 'asm': operand number out of range ../../../../../rtems-4.10.99-src/c/src/libchip/sdmmc/ald-sd-card.c:159:1: error: invalid 'asm': invalid operand output code – Chan Kim Sep 06 '15 at 08:26
0

It appears you are using GCC.

As @MikeCAT said the instructions need to be on separate lines.

You need to double all the % in register names. %% becomes % in the output assembly.

nr is in %o0, addr in %o1 by sparc rule

There is no such rule, you might be thinking of the function call ABI which does not apply to inline assembly. GCC is expecting addr to be written to the memory location %0 and nr is in the memory location %1 as you asked:

: "=m"(addr) // output
: "m"(nr) // input

But addr is not an output. Either *addr is an input/output or addr is an input and "memory" must be in the clobber list. A register would be a better place for nr:

: "+m"(*addr) // input and output
: "r"(nr) // input

You should not put retl in there as it jumps to an undefined location, control flow should reach the end.

You must list all changed registers either as outputs or clobbers.

If this is supposed to be a memory barrier "memory" must be in the clobber list.

I refer you to the GCC Manual.

Edit: missed a pointer dereference.

Timothy Baldwin
  • 3,551
  • 1
  • 14
  • 23