[libunwind] Add support for ARMv7-M architecture which uses the Thumb 2 ISA (unified syntax)

[jeremfg]

Summary

I'm a first time contributor to LLVM...

While attempting to compile the assembly files ( .S) for a Cortex-M4 device using GCC's arm-none-eabi, I encountered issue which this patch fixes.

[mstorsjo]

The change looks sensible to me, but should we maybe even skip the #if altogether? If the files uses unified syntax and can't be parsed in thumb mode otherwise, there's maybe no need for conditionals at all?

[jeremfg]

In D60417#1458778, @mstorsjo wrote:

The change looks sensible to me, but should we maybe even skip the #if altogether? If the files uses unified syntax and can't be parsed in thumb mode otherwise, there's maybe no need for conditionals at all?

I'm no expert but I just read that UAL (Unified Assembler Language), while being backward compatible with the old ARM syntax, is not compatible with the previous Thumb syntax.
http://downloads.ti.com/docs/esd/SPNU118/unified-assembly-language-syntax-support-spnu1184444.html

But again, perhaps it doesn't matter in the specific case that concerns us here? I guess it depends on whether the code in the #if !defined(__ARM_ARCH_ISA_ARM) && __ARM_ARCH_ISA_THUMB == 1 condition that follows contains only valid UAL syntax as well. Which I am not qualified to confirm or not.

If somebody can confirm that my understanding is correct, I could try and see if GCC throws an error during compilation for the ARMv6-M architecture, but I don't trust myself enough to rely on my own tests alone and my weak understanding to ensure I won't break support for current Thumb-1 only users. Hopefully somebody else can pitch in to confirm (or infirm) my understanding.

By the way, are there other architecture besides ARMv6-M that doesn't support ARM and report that __ARM_ARCH_ISA_THUMB == 1 ?

If that's all UAL-compatible instructions, then yes I think my #if could be removed and only the .syntax unified should be kept. At least, that's how I understand it. I could be wrong...
Perhaps some assemblers simply refuses the .sytanx unified directive altogether when targeting legacy Thumb-1 only architectures, making this all a moot discussion?

[mstorsjo]

In D60417#1459128, @jeremfg wrote:

In D60417#1458778, @mstorsjo wrote:

The change looks sensible to me, but should we maybe even skip the #if altogether? If the files uses unified syntax and can't be parsed in thumb mode otherwise, there's maybe no need for conditionals at all?

I'm no expert but I just read that UAL (Unified Assembler Language), while being backward compatible with the old ARM syntax, is not compatible with the previous Thumb syntax.
http://downloads.ti.com/docs/esd/SPNU118/unified-assembly-language-syntax-support-spnu1184444.html

But again, perhaps it doesn't matter in the specific case that concerns us here? I guess it depends on whether the code in the #if !defined(__ARM_ARCH_ISA_ARM) && __ARM_ARCH_ISA_THUMB == 1 condition that follows contains only valid UAL syntax as well. Which I am not qualified to confirm or not.

Oh, sorry, I was sloppy and didn't actually check the rest of the file to see the existing conditional alternative codepaths with !defined(__ARM_ARCH_ISA_ARM) && __ARM_ARCH_ISA_THUMB == 1.

Given that, the suggested form of this patch is indeed correct - sorry for the noise.

I can commit the patch for you, but I'd need approval by either of @EricWF, @ldionne or @mclow.lists first.

[mstorsjo]

In D60417#1459249, @mstorsjo wrote:

Given that, the suggested form of this patch is indeed correct - sorry for the noise.

I can commit the patch for you, but I'd need approval by either of @EricWF, @ldionne or @mclow.lists first.

Ping @EricWF @ldionne @mclow.lists

[mstorsjo]

In D60417#1464070, @mstorsjo wrote:

In D60417#1459249, @mstorsjo wrote:

Given that, the suggested form of this patch is indeed correct - sorry for the noise.

I can commit the patch for you, but I'd need approval by either of @EricWF, @ldionne or @mclow.lists first.

Ping @EricWF @ldionne @mclow.lists

Ping @EricWF @ldionne @mclow.lists

[ldionne]

I'm relaying approval to @mstorsjo, who seems okay with the change.

[mclow.lists]

I have no problem with this.

libunwind/trunk/src/UnwindRegistersRestore.S

DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind15Registers_arm646jumptoEv)
   ldr    d31,     [x0, #0x208]
 
   ldp    x0, x1,  [x0, #0x000]  // restore x0,x1
   ret    x30                    // jump to pc
 
 #elif defined(__arm__) && !defined(__APPLE__)
 
 #if !defined(__ARM_ARCH_ISA_ARM)
+#if (__ARM_ARCH_ISA_THUMB == 2)
+  .syntax unified
+#endif
   .thumb
 #endif
 
 @
 @ void libunwind::Registers_arm::restoreCoreAndJumpTo()
 @
 @ On entry:
 @  thread_state pointer is in r0

libunwind/trunk/src/UnwindRegistersSave.S

DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext)
   str    d30,     [x0, #0x200]
   str    d31,     [x0, #0x208]
   mov    x0, #0                   // return UNW_ESUCCESS
   ret
 
 #elif defined(__arm__) && !defined(__APPLE__)
 
 #if !defined(__ARM_ARCH_ISA_ARM)
+#if (__ARM_ARCH_ISA_THUMB == 2)
+  .syntax unified
+#endif
   .thumb
 #endif
 
 @
 @ extern int __unw_getcontext(unw_context_t* thread_state)
 @
 @ On entry:
 @  thread_state pointer is in r0