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.
Differential D60417
[libunwind] Add support for ARMv7-M architecture which uses the Thumb 2 ISA (unified syntax) jeremfg on Apr 8 2019, 12:03 PM. Authored by
Details
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.
Diff Detail
Event TimelineComment Actions 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? Comment Actions 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. 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... Comment Actions 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. |