Add CThunk for CM7

[svastm]

The asynchronous versions of the communication interfaces (serial, i2c and spi) need the CThunk class which is not yet implemented for cortex-M7.

My first idea was to use the same OP code than CM3 and CM4, ldm.w pc,{r0,r1,r2,pc}, because they use ARMv7-M and thumb2 like the CM7. But this OP code give me an hard fault and I don't understand why.

I poorly used the code from CM0, CM0+ which looks cleaner but a bit longer. Then I was able to run the following test code on ARM and IAR but it still fail on GCC. I tried with a NUCLEO_F746ZG and a DISCO_F746NG.

#include "mbed.h"
#include "CThunk.h"

class DummyClass {
  public: 
    void method() {
        printf("method called => attribute: %d\n", attribute);
    }

  public:
    int attribute;
};

int main() {
    printf("Begin\n");

    DummyClass dm;
    CThunk<DummyClass> ct = CThunk<DummyClass>(&dm, &DummyClass::method);

    dm.attribute = 0;
    ct.call();

    dm.attribute += 1;
    ct.call();

    printf("End\n");

    while (1) {

    }
}

I'm a bit stuck there and i can't find something in the ARMv7-M manual which say to not use the ldm.w pc,{r0,r1,r2,pc}.

Someone has already looked at this ?

[0xc0170]

My first idea was to use the same OP code than CM3 and CM4, ldm.w pc,{r0,r1,r2,pc}, because they use ARMv7-M and thumb2 like the CM7. But this OP code give me an hard fault and I don't understand why.

cc @c1728p9

[svastm]

Then in this manual p 3-35, LDM PC, xxx is forbidden on CM7. So we can't use the short code.

[svastm]

Rebased with a fully working version. GCC was failing because this toolchain define the __thumb2__ macro. So it was still using the short trampoline.

[bogdanm]

Cc @meriac for any suggestions. According to the manual for Cortex-M4, Cortex-M3 and Cortex-M7, LDM can't accept pc as the destination, and yet this seems to work on M4 and M3. I don't know what happens in M7.

[c1728p9]

Since the documentation for M3 and M4 don't allow the PC to be used to as the address to load from in a LDM instruction should all the targets be using the M0 implementation? @meriac, what do you think?

[0xc0170]

Since the documentation for M3 and M4 don't allow the PC to be used to as the address to load from in a LDM instruction should all the targets be using the M0 implementation? @meriac, what do you think?

Looks like that restriction was put into practise for m7 cores (hardfaults). If we don't find a better alternative (like not touching anything just scratch registers), we might just use thumb instruction for all cores. I can't think of anything better at the moment. You guys?

[geky]

Was toying around with this a while back, here's a small thumb trampoline that avoids undefined instructions:

0xa002 // add r0, pc, #8
0xc803 // ldm r0, {r0, r1}
0x4708 // bx r1
0x0000 // make sure context and callback are aligned
context
callback

EDIT: I realized the above was a 1-argument thunk. The CThunk class needs a 3-argument thunk. An alternative is to modify the CThunk class to emit a single-argument static function (example), which may make writing efficient thunks easier in the future.

0xa002 // add r0, pc, #8
0xc80f // ldm r0, {r0, r1, r2, r3}
0x4718 // bx r3
0x0000 // make sure context and callback are aligned
context
instance
callback
trampoline

It's kinda sad to see the ldm.w pc,{r0,r1,r2,pc} go (it was a very neat trick), but it is probably for the better if it was undefined behaviour.

[svastm]

This solution looks great to me. I don't think we can do shorter. Because it use only thumb instruction set, we could also use it for CM0/CM0+.
Should i rebase it ?

[bogdanm]

@meriac @AlessandroA : if you have something against this, speak now :)

[AlessandroA]

The only reason why I could imagine a different behavior between the cores is because of this condition not passing (from ARM DDI 0403E.b (ARMv7M ARM) page A7-248):

n = UInt(Rn); registers = P:M:'0':register_list; wback = (W == '1');
if n == 15 || BitCount(registers) < 2 || (P == '1' && M == '1') then UNPREDICTABLE;
if registers<15> == '1' && InITBlock() && !LastInITBlock() then UNPREDICTABLE;

Where n == 15 means pc. The unpredictable behavior is probably different between M3/M4 and M7.

The following instructions would be an alternative to what @geky proposed:

add r0, pc, #8
ldm r0, {r0, r1, r2, pc}

You don't need to take pc out of the register list, that is allowed and implicitly turns into a branch. The footprint is the same though, and you still need alignment.

[0xc0170]

The above code snippet looks good, saves 4 bytes :) @AlessandroA thanks for looking at this. Same footprint, but we might want unified version.

@svastm Can you test locally please and rebase?

[bogdanm]

Thanks @AlessandroA! LGTM.

[svastm]

I rebase with the new implementations thanks to @geky and @AlessandroA.

I kept the geky solution for thumb implementation because the ldm thumb instruction can use only low register.
Also, i remove the __thumb2__ macro check because it was only defined by GCC_ARM.
I tested with a NUCLEO_F091RC, NUCLEO_F303RE and NUCLEO_F746ZG. For ARM, GCC_ARM and IAR it's ok.

For Cortex-A9 i switched to thumb2 solution but I don't have a Cortex-A9 so I can't test it.

[geky]

Tested locally with the Cortex-A9 RZ_A1H, looks good to me: 👍

Begin
method called => attribute: 0
method called => attribute: 1
End

[0xc0170]

/morph test

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 850

All builds and test passed!