#649 atomic_flag

[Philipp Klaus Krause]

It would be good to have support for C11 atomic_flag as an atomic type that can be used in interrupt handlers.

For architectures that have an atomic exchange, such as stm8, this should be easy.
For architectures without, a shift with indirect addressing mode could help.
On other architectures it will be harder to implement.

[Sergey Belyashov]

Interesting, how to implement atomic_flag_test_and_set on z80? Imho, inc (hl) is more useful here:

typedef struct atomic_flag { signed char cnt; } atomic_flag;
#define ATOMIC_FLAG_INIT {-1}
_Bool atomic_flag_test_and_set(volatile atomic_flag* obj) __naked
{
    __asm
    pop bc
    pop hl
    push hl
    push bc
    xor a, a
    inc (hl)
    jr z,10$ ;aquired
    dec (hl)
    inc a
10$:
    ld l,a
    ret
    __endasm;
}
void atomic_flag_clear(volatile atomic_flag* obj) __naked
{
    __asm
    pop bc
    pop hl
    push hl
    push bc
    dec (hl)
    ret
    __endasm;
}

[Philipp Klaus Krause]

I don't see how that could work. Doesn't look atomic either.

But using inverted meanings of 1 and 0, srl (hl) for test_and_set would work: It atomically sets to 0 (no matter if the value was 0 or 1 before), and gets us the previous values (in the carry flag).

[Sergey Belyashov]

heh. your way is simpler and safer

[Philipp Klaus Krause]

As of [r11552], there is now basic support for atomic_flag for the z80, gbz80, r2k and stm8 ports.
Actually, the atomic exchange in stm8 does not have an indirect addressing mode, so stm8 uses the same idea as z80 instead.

[Philipp Klaus Krause]

As of [r11554], this is implemented for most ports. So far all use the shift approach.

Still missing:

mcs51 - apparently has a proper atomic exchange, so could be done using that instead of shift.
ds390 - didn't look into this one yet.
pic14, pic16 - don't really want to look into those.
tlcs90 - So many choices. Could use the shift approach. Or the tset instruction. Or the 16-bit atomic exchange.
pdk13, pdk14, pdk15 - no good option, will require very complicated workarounds

[Sergey Belyashov]

Thank you.
Why do you copy same code between targets? Is it better to use one generic *.c for all related targets and select required instructions in naked function by #ifdef?
What is reason to ignore __z88dk_fastcall for standard library routines? gcc/clang have many conventions for support library routines, for example.

[Philipp Klaus Krause]

Regarding the copies:

There are two ways to handle library functions htat differ between some ports:
1) Have a version of the function for each port - results in code duplication for closely related ports.
2) Have a common file, with #ifdef to select the version - makes it hard to read, and places port-specific code in places where it shouldn't be.

The solution would be to have some way to designate some port as a parent of the other, so e.g. z80 would be parent for all z80-related, hc08 would be parent of s08, etc.
Then, by default the port-specific version would be used, and one would next fall back to the parent port version. But no such mechanism is currently in place.

[Philipp Klaus Krause]

Standard library functions need to use the default calling convention, so
1) Pointers to them can be assigned to user-declared function pointers
2) The user can use their own declarations instead of standard headers.

[Sergey Belyashov]

yeh... still waiting for [feature-requests:#253]

[Philipp Klaus Krause]

For tlcs90:

• The atomic exchange would be the fastest and smallest code for implementing atomic_flag_test_and_set, but it would require atomic_flag to be a 16-bit type.
• Otherwise, the shift approach is the same size as the tset, and a bit faster.
• atomic_flag_test_and_set is often used in conditions, and for this case tset is perfect, but that would require atomic_flag_test_and_set to be a builtin, or an iCode.

[Sergey Belyashov]

you may create __atomic_test_set built-in function and then use it in public implementation like memcpy.

[Sergey Belyashov]

Implemented library function in [r11814].

[Philipp Klaus Krause]

For mcs51, there is a problem:

Implementation of atomic_flag there can be done using:

xch a, @ri

This is both the only feasible and a very efficient way of implementing atomic_flag for MCS-51.

This instruction uses indirect addressing mode. Thus, atomic_flag has to always live in the address space __idata.

A first idea might be to just use __idata in the typedef for atomic_flag. However, atomic_flag should also be useable as a struct / union member. And members can't be qualified by named address spaces.

[Maarten Brock]

Maybe we can change the prototype for mcs51 like this:

_Bool atomic_flag_test_and_set(volatile atomic_flag __idata *object);

This would allow the atomic_flag to be part of a struct as long as this struct lives in __idata.

If we want to support atomic_flag in __xdata / __pdata or through a generic pointer as well we would have to write separate implementations and decide at compile time which one to use. The generic implementation would be very sub-optimal and require disabling interrupts.

[Maarten Brock]

Implemented for mcs51 in [r11771].

[Maarten Brock]

And fixed in [r11778]. I forgot some files.
Further it is now also implemented for ds390.

[Maarten Brock]

Would it be legal to make atomic_flag_clear() an inline definition in stdatomic.h and place an external definition in the libs?

[Philipp Klaus Krause]

I think that would work.

[Philipp Klaus Krause]

As of [r14918], the only maintained ports without support for atomic_flag are the pdk ports.