Volatile atomic

[dpetrisko]

I've been encountering an issue mixing loads and amos to the same address. I've convinced myself it's a coding issue rather than a hardware issue, with the compiler not tracking the inline assembly dependencies correctly. Making the pointer volatile seems to make the accesses sane. But then compiling with C++ -fno-permissive, causes the following:

main.c:27:24: error: invalid conversion from 'volatile int*' to 'int*' [-fpermissive]
   27 |     int y = bsg_amoadd(vol_lock_ptr, 1);
      |                        ^~~~~~~~~~~~
      |                        |
      |                        volatile int*

This PR makes the pointer parameter for the bsg_amo_add volatile so that we can compile this code correctly. It also adds a test demonstrating the issue

[dpetrisko]

@mrutt92 foresee any issues doing this?

[mrutt92]

It seems reasonable to me. Adding the volatile modifier to the pointer is typical for defining functions like these.

If you're writing c++ I encourage using std::atomic if you can... then you don't need to think about this volatile nonsense.

Could you post some example C and assembly showing the bug?

[dpetrisko]

Ya the test added will fail if you switch out "int lock_ptr = &lock;" for "volatile int lock_ptr = &lock;"

#include "bsg_manycore.h"
#include "bsg_set_tile_x_y.h"
#include "bsg_manycore_atomic.h"

int lock __attribute__ ((section (".dram"))) = {0};

int main()
{

  bsg_set_tile_x_y();

  if (__bsg_id == 0)
  {
    // Using a regular int* here cause an illegal ordering resulting in:
    //   x = 1, y = 0, z = 1
    //int* lock_ptr = &lock;
    volatile int* lock_ptr  = &lock;
    int x = *lock_ptr;
    int y = bsg_amoadd(lock_ptr, 1);
    int z = *lock_ptr;

    if (x != 0 || y != 0 || z != 1) bsg_fail();

    bsg_finish();
  }

  bsg_wait_while(1);
}

[dpetrisko]

This might be more informative, you can see the difference in ASM here:

#include "bsg_manycore.h"
#include "bsg_set_tile_x_y.h"
#include "bsg_manycore_atomic.h"

int nvol_lock __attribute__ ((section (".dram"))) = {0};
int vol_lock __attribute__ ((section (".dram"))) = {0};

int main()
{

  bsg_set_tile_x_y();

  if (__bsg_id == 0)
  {
    int* nvol_lock_ptr = &nvol_lock;
    int q = *nvol_lock_ptr;
    int r = bsg_amoadd(nvol_lock_ptr, 1);
    int s = *nvol_lock_ptr;

    volatile int* vol_lock_ptr  = &vol_lock;
    int x = *vol_lock_ptr;
    int y = bsg_amoadd(vol_lock_ptr, 1);
    int z = *vol_lock_ptr;

    bsg_print_hexadecimal(q);
    bsg_print_hexadecimal(r);
    bsg_print_hexadecimal(s);

    bsg_print_hexadecimal(x);
    bsg_print_hexadecimal(y);
    bsg_print_hexadecimal(z);

    bsg_finish();
  }

  bsg_wait_while(1);
}

000001c8 <main>:
 1c8:   ff010113            addi    sp,sp,-16
 1cc:   00112623            sw  ra,12(sp)
 1d0:   f59ff0ef            jal ra,128 <bsg_set_tile_x_y>
 1d4:   02c02283            lw  t0,44(zero) # 2c <__bsg_id>
 1d8:   04029e63            bnez    t0,234 <main+0x6c>
 1dc:   81000737            lui a4,0x81000
 1e0:   06070093            addi    ra,a4,96 # 81000060 <_bsg_dram_end_addr+0xfffffff0>
 1e4:   00100313            li  t1,1
 1e8:   0060a52f            amoadd.w    a0,t1,(ra)
 1ec:   0040a583            lw  a1,4(ra)
 1f0:   00408693            addi    a3,ra,4
 1f4:   0066a3af            amoadd.w    t2,t1,(a3)
 1f8:   0000a603            lw  a2,0(ra)
 1fc:   4010f837            lui a6,0x4010f
 200:   0040a883            lw  a7,4(ra)
 204:   aec82423            sw  a2,-1304(a6) # 4010eae8 <_bsg_elf_vcache_size+0x400ceae8>
 208:   aea82423            sw  a0,-1304(a6)
 20c:   aec82423            sw  a2,-1304(a6)
 210:   aeb82423            sw  a1,-1304(a6)
 214:   ae782423            sw  t2,-1304(a6)
 218:   af182423            sw  a7,-1304(a6)
 21c:   03002e83            lw  t4,48(zero) # 30 <__bsg_y>
 220:   03402f83            lw  t6,52(zero) # 34 <__bsg_x>
 224:   010e9793            slli    a5,t4,0x10
 228:   01f782b3            add t0,a5,t6
 22c:   ac582823            sw  t0,-1328(a6)
 230:   0000006f            j   230 <main+0x68>
 234:   0000006f            j   234 <main+0x6c>