[PATCH] [lbc++abi] [unwinder] Support Cortex-m0

Jonathan Roelofs jonathan at codesourcery.com
Fri Sep 12 05:47:51 PDT 2014


Sorry about the hideously bad merge....

================
Comment at: src/Unwind/UnwindRegistersRestore.S:341
@@ -330,1 +340,3 @@
+#endif
+#if __ARM_ARCH > 4 || __ARM_ARCH_4T__
   bx lr
----------------
rengolin wrote:
> I think we should use the same trick we used in other ARM files, to define a macro that is set by the arch for returning.
Sounds like a good idea to me.

================
Comment at: src/Unwind/UnwindRegistersRestore.S:355
@@ -342,2 +354,3 @@
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm19restoreVFPWithFLDMDEPy)
+#if !defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)
   @ VFP and iwMMX instructions are only available when compiling with the flags
----------------
rengolin wrote:
> Do you want to export the symbols (unwindVFP) but have it do nothing? Is it even possible to try to unwind VFP on v6M? Shouldn't this be a compilation error?
I think that this will never get called on v6m because it doesn't have vfp registers, so the compiler will never issue the unwind opcodes that trigger this being called. Maybe that warrants having the body be: 
```
assert(false)
```
?


This could be completely ifdef'd out on v6m, but that would mean ifdefing out the callers too, which would be a bit ugly. It does save a few bytes though....

================
Comment at: src/Unwind/UnwindRegistersRestore.S:380
@@ -365,2 +379,3 @@
 #if __ARM_ARCH < 7
+#if !defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)
   ldc p11, cr0, [r0], {0x21}  @ fldmiax r0, {d0-d15}
----------------
rengolin wrote:
> This macro is in a different order than the one above, and will generate vldmia on v6M which will fail.
Oops, bad merge. Yeah, they should be transposed.

================
Comment at: src/Unwind/UnwindRegistersRestore.S:397
@@ -380,2 +396,3 @@
 #if __ARM_ARCH < 7
+#if !defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)
   ldcl p11, cr0, [r0], {0x20}  @ vldm r0, {d16-d31}
----------------
Another bad merge. Transpose this with the line above it.

================
Comment at: src/Unwind/UnwindRegistersRestore.S:440
@@ -419,2 +439,3 @@
   .p2align 2
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm19restoreiWMMXControlEPj)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm19restoreiWMMXControlEPm)
+#if (!defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)) || __ARM_WMMX
----------------
Another bad merge / patch cut. I didn't intend to include the s/MMXControlEPj/MMXControlEPm/ change.

================
Comment at: src/Unwind/UnwindRegistersSave.S:285
@@ -284,1 +284,3 @@
 
+#if defined(__ARM_ARCH_6M__) || defined(__ARM_ARCH_6SM__) 
+  .thumb
----------------
rengolin wrote:
> Maybe we should also have a macro that will use thumb on thumb-only platforms like v6M and v6M variants. That would have to be accompanied by changes to other areas, making them Thumb/2 friendly (like the ones you add here).
like a LIBUNWIND_THUMB_ONLY flag?

================
Comment at: src/Unwind/UnwindRegistersSave.S:335
@@ -321,2 +334,3 @@
 #if __ARM_ARCH < 7
+#if defined(__ARM_ARCH_6M__) || defined(__ARM_ARCH_6SM__) 
   stc p11, cr0, [r0], {0x20}  @ fstmiad r0, {d0-d15}
----------------
compnerd wrote:
> Doesn't this change the behavior for __ARM_ARCH >= 7?
Another bad merge. This line, and the one above it should be transposed.

================
Comment at: src/Unwind/UnwindRegistersSave.S:352
@@ -336,2 +351,3 @@
 #if __ARM_ARCH < 7
+#if defined(__ARM_ARCH_6M__) || defined(__ARM_ARCH_6SM__) 
   stc p11, cr0, [r0], {0x21}  @ fstmiax r0, {d0-d15}
----------------
compnerd wrote:
> Doesn't this change the behavior for __ARM_ARCH >= 7?
Another bad merge. This line, and the one above it should be transposed.

================
Comment at: src/Unwind/UnwindRegistersSave.S:368
@@ -350,2 +367,3 @@
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm9saveVFPv3EPy)
+#if defined(__ARM_ARCH_6M__) || defined(__ARM_ARCH_6SM__) 
   @ VFP and iwMMX instructions are only available when compiling with the flags
----------------
rengolin wrote:
> compnerd wrote:
> > This also doesn't work for ARMv7.  Consider ARM VFPv3-d16 (e.g. Tegra2) ... vstmia r0, {d16-d31} refers to a non-existent register set.
> And this is exactly the opposite as the other, which would just get "bx lr" on ARMv6M
Conditions on this one should be flipped.... oops.

================
Comment at: src/Unwind/UnwindRegistersSave.S:382
@@ -363,2 +381,3 @@
+#endif
   mov pc, lr
 
----------------
Maybe this should use that RETURN macro @rengolin suggests?

================
Comment at: src/Unwind/UnwindRegistersSave.S:392
@@ -372,2 +391,3 @@
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm9saveiWMMXEPy)
+#if (!defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)) || __ARM_WMMX
   stcl p1, cr0, [r0], #8  @ wstrd wR0, [r0], #8
----------------
compnerd wrote:
> Do all CPUs have the wmmxt register set?  It might be more prudent to move this out into a separate translation unit that is linked in if the target supports iwmmxt.
I think the idea here (before my change) was that if you were to build this library with `-march=armv5t` then you could either link it with code for a PXA270 and get something that would work on that arch, or link it with normal v5t code and get something that would work on a vanilla v5t board.

With my change I am trying to be conservative... I don't know whether there are any v6m's with iwmmx support, but if there are, then this should DTRT.

================
Comment at: src/Unwind/UnwindRegistersSave.S:419
@@ -397,2 +418,3 @@
   .p2align 2
-DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm16saveiWMMXControlEPj)
+DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm16saveiWMMXControlEPm)
+#if (!defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)) || __ARM_WMMX
----------------
compnerd wrote:
> Oh joy ... this is scary.
> 
> Do we have some guarantee that they won't accidentally use long rather than int?
Oops, another bad patch cut. The s/saveiWMMXControlEPj/saveiWMMXControlEPm/ change is part of a different patch where I change the type of the argument to void*.

http://reviews.llvm.org/D5314






More information about the cfe-commits mailing list