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

[Weiming Zhao via Phabricator via llvm-commits]

weimingz added inline comments.


================
Comment at: lib/builtins/arm/udivsi3.S:106
+	adr r0, LOCAL_LABEL(div0block)
+	adds r0, #1
+#    else
----------------
compnerd wrote:
> weimingz wrote:
> > compnerd wrote:
> > > Can we use or instead?
> > in Thumb1, ORR doesn't support immediate values.
> Hmm, do we have some way to guarantee that the thumb bit would never be set by the linker?  (at least with link, the linker will set the thumb bit on the relocation target if it is in the code section on a thumb block).
this is the same as the thumbe2 version (lne70-71)
#    if __ARM_ARCH_ISA_THUMB == 2
  adr  ip, LOCAL_LABEL(div0block) + 1

Since it's PC-relative, there should be no relocation entry for this.



================
Comment at: lib/builtins/arm/udivsi3.S:200
+LOCAL_LABEL(block_skip_ ## shift): ;\
+	adcs r3, r3 /* same as ((r3 << 1) | Carry). Carry is set if r0 >= r2. */
+
----------------
compnerd wrote:
> Can you format this please?  (clang-format).
Will do. I thought clang-format was only for C/C++.


================
Comment at: lib/builtins/arm/udivsi3.S:254
 
+#if __ARM_ARCH_ISA_THUMB != 1
 LOCAL_LABEL(divby0):
----------------
compnerd wrote:
> Any reason to not just use the single definition of this label?  (as in just move it for both cases.
Right, we don't need two definitions. Will remove this.


https://reviews.llvm.org/D27309