Using Clang and the S110 (issues with supervisor calls to the SoftDevice)

[Moloch]

Hi everyone,

I've been experimenting compiling my project with Clang and have run into some issues with the supervisor calls and their wrapper in nrf_svc.h.

Without optimizations, the calls seem to work fine. See the following asm generated by Clang at -O0 that loads r0 and r1 with the parameters for the SVC:

/* uint32_t  rc = sd_softdevice_enable(NRF_CLOCK_LFCLKSRC_XTAL_75_PPM, assertion_handler); */
ldr r1, 0x000188A8 // second parameter <= &assertion_handler()
movs r0, #5 // first parameter <= LFCLK configuration
bl 0x000188B8 <sd_softdevice_enable>
ldr r1, [sp, #16]  // Put address of 'rc' into r1
str r0, [r1] // Store return value from SVC into 'rc'

/* ASSERT(rc == NRF_SUCCESS); */
cmp r0, #0 // Check 'rc == NRF_SUCCESS'
beq 0x00018810 // All good, don't assert
b 0x00018802 // Assert, rc != NRF_SUCCESS

<sd_softdevice_enable>
add r2, sp, #8
mov r3, r1
mov r4, r0
str r0, [r2, #4]
str r1, [r2]

svc #16
bx lr

There are some unnecessary moves and stores in <sd_softdevice_enable> (since everything is already in a register), but it appears to work.

To contrast, here's equivalent asm from GCC at -O2 (much more concise):

/* uint32_t rc = sd_softdevice_enable(NRF_CLOCK_LFCLKSRC_XTAL_75_PPM, assertion_handler); */
movs r0, #5 // first parameter <= LFCLK configuration
ldr r1, 0x00018938 // second parameter <= &assertion_handler()
bl 0x000187BC <sd_softdevice_enable>

/* ASSERT(rc == NRF_SUCCESS); */
cmp r0, #0 // Check 'rc == NRF_SUCCESS'
bne 0x00018928 // Assert if !=

<sd_softdevice_enable>
svc #16
bx lr

And here's Clang again at -O2. Everything breaks down.

/* uint32_t rc = sd_softdevice_enable( NRF_CLOCK_LFCLKSRC_XTAL_75_PPM, assertion_handler); */
bl 0x0001889C <sd_softdevice_enable> // Where are the parameter loads into r0 and r1?

// Oh no! It unconditionally asserts!
/* ASSERT(rc == NRF_SUCCESS); */
 ldr r4, 0x0001888C
 ldr r5, 0x00018890
 movs r2, #0x72
 mov r0, r4
 mov r1, r5
 bl 0x000254F4 <uart_fatalOut>
 bkpt #0 // R.I.P.

<sd_softdevice_enable> // Same as the previous two
svc #16
bx lr

Summarized, here's what I see at Clang -O2:

1. No loading of r0, r1 of the input parameters to the SVC
2. No capturing of the return value from sd_softdevice_enable

It's as if Clang isn't following the calling convention.

Here's my hypothesis of why. The following is from nrf_svc.h (just the relevant part):

#define SVCALL(number, return_type, signature) \
__attribute__((naked)) static return_type signature \
{ \
    __asm( \
        "svc %0\n" \
        "bx r14" : : "I" (number) : "r0" \
    ); \
}

Nordic uses the preprocessor macro SVCALL to define all the sd_* functions implemented as supervisor calls. Here's the definition of sd_softdevice_enable after preprocessing and some improved formatting:

 __attribute__((naked)) static uint32_t sd_softdevice_enable(
    nrf_clock_lfclksrc_t clock_source,
    softdevice_assertion_handler_t assertion_handler)
{
    __asm( 
        "svc %0 \t\n
         bx r14 \t\n"
         : : "I" (SD_SOFTDEVICE_ENABLE) : "r0"
    );
}

Maybe I'm wrong, but I understand that using extended inline assembly is undefined in a naked function. Only basic inline asm, without output:input:clobber lists is allowed. GCC's docs say so here.

Only basic asm statements can safely be included in naked functions (see Basic Asm). While using extended asm or a mixture of basic asm and C code may appear to work, they cannot be depended upon to work reliably and are not supported.

It seems to me that GCC does the "right" thing, and Clang doesn't, however relying on GCC to always do the right thing isn't dependable.

I'm not very experienced at this low-level, but I think that using standard C functions (dropping the static and the naked attribute) might be the solution.

Of course, I could be misinterpreting the problem. I'm interested in hearing any comments to that effect.

Thanks!

[RK]

And this llvm.org/.../show_bug.cgi is possibly one of the bugs I mentioned which added extra code and broke the stack. That claims to have been fixed in September and I'm assuming Crossworks has upgraded to a later version of Clang since then. So we may just be left with the register-optimise-away issue.

[Bill Siever]

Thanks! I had only been looking at the clang assembly versions which had significant overhead due to the stack setup/teardown. Soon after posting I realized that an in-line version is preferable.

[Bill Siever]

clang is destroying the stack with the "naked" version. By the way, it looks like I re-did the same two approaches you followed (a tedious hand-coded approach and an attempt at utilizing SVCALL macros).

[Bill Siever]

I made an update that seems to generate more compact code:

    #define SVCALL2(number, rettype, signature, p1, p2)  \
__attribute__((always_inline)) static inline rettype signature {  \
    register typeof(p1) _p1 asm("r0") = p1;         \
    register typeof(p2) _p2 asm("r1") = p2;         \
    asm volatile (                                  \
       "svc %[i]\n\t"                               \
       : "=r" (_p1)                                 \
       :[i] "i" (number), [a] "r" (_p1), [b] "r" (_p2) \
       : "r0");                                     \
    return (rettype)_p1;                            \
}

Now the parameters are also used as the return values. My test case generated two fewer instructions. Moloch's sd_softdevice_enable(a,b):

movs    r0, #5
ldr     r1, .LCPI3_26
svc     #16
cmp     r0, #0

[RK]

That looks pretty similar to the code I ended up with last time I tried. You found the same issue I did, mentioned in my earlier reply, that the way SVCCALL is defined makes it hard to automate, you, like me, made a SVCCALL2, SVCCALL3 .. etc to work around that issue. I don't know if we can sell Nordic on that idea just to fix Clang.

I also tried a pure assembly .s file but that didn't work because all the SD_ values are in an enum, not #defines. That works in a .c file with embedded assembler because the c-compiler eventually turns the enum values into fixed numbers which then make the 'svc xxx' instruction work, however the assembler can''t do that, because it doesn't know what enums are.

This thread reminds me how frustrating it was. Shame, I'd like to fix it as I like some of the code clang generates. I'm thinking preprocessing the .h file may be the only way to approach this.