[PATCH] D27309: builtins: Add ARM Thumb1 implementation for uidiv and uidivmod

[Weiming Zhao via Phabricator via llvm-commits]

weimingz added a comment.

In https://reviews.llvm.org/D27309#698801, @strejda wrote:

> In https://reviews.llvm.org/D27309#698696, @weimingz wrote:
>
> > Most of the changes are guarded by __ARM_ARCH_ISA_THUMB == 1 and I reviewed again of the "else" part and they look OK except for one suspect:
> >  https://reviews.llvm.org/D30867
>
>
> No, unfortunately, _ _ARM_ARCH_ISA_THUMB == n can't be used as selector for Thumb1 (or Thumb2) code. 
>  Value of  _ _ARM_ARCH_ISA_THUMB isn't based on actual compilation  mode (-mthumb, -marm), its based on given CPU (or arch) capabilities.
>  Thus, if we compile library with '-marm  -march=armv6' then _ _ARM_ARCH_ISA_THUMB is set to '1'. So we ends with Thumb1 code compiled using ARM ISA. Not a nice result, right?
>  So https://reviews.llvm.org/D30867 cannot help us.
>
> I found these problems:
>
> - Thumb[1][2] code selection is broken,   "(_ _ARM_ARCH_ISA_THUMB == n && (defined(_ _THUMBEL_ _) || defined(_ _THUMBEB_ _))" is right condition. See: https://github.com/strejda/tegra/commit/91057bb46e54e350da87e354aed3189d469da458#diff-a8e5a4d82eadfcc89b4f54d6755a46b4L97
>
>   This one is fatal for FreeBSD.
> - all Thumb1  and some Thumb2 functions are not properly decorated by DEFINE_COMPILERRT_THUMB_FUNCTION() This breaks debugging.
> - usage of '.thumb' directive is inconsistent  across files
> - Thumb 1 version of __udivsi3 is broken and doesn't not work at all. See: ``` 	cmp	r0, r1, lsl IMM shift 	addhs	r3, r3, IMM (1 << shift) 	subhs	r0, r0, r1, lsl IMM shift ``` and
>
>   ``` 	lsls	r2, r1, IMM shift 	cmp	r0, r2 	blo	1f 	subs	r0, r0, r2 1: 	adcs	r3, r3 ```
> - There is no Thumb1 variant of __udivmodsi4


Great! Thanks for finding out the root cause. Will you pull the github patch into upstream repo? 
Regarding udivmodi4.S, we can work on it. Currently, the thumb1 lib can use the C version (udivmodsi4.c ), which simply calls __udivsi3 with extra mul and sub . Since we implemented _udivsi3 in asm, the perf won't be too bad.


Repository:
  rL LLVM

https://reviews.llvm.org/D27309