[compiler-rt] r215295 - builtins: correct __umodsi3, __udivsi3 on ARM

[Renato Golin]

Saleem,

I agree that __umodsi3 was completely wrong, and I believe this is a
left-over from an old divmod implementation, as is the __udivsi3.
Anton would know better, as I believe he wrote the original code.

I'm worried about the bogus write to [r2] as it could really lead to
memory corruption, but not so much for the udiv.

cheers,
--renato


On 18 August 2014 06:29, Bill Wendling <isanbard at gmail.com> wrote:
> Jim & Owen
>
> What do you think?
>
> -bw
>
> On Aug 10, 2014, at 1:06 PM, Saleem Abdulrasool <compnerd at compnerd.org>
> wrote:
>
> Hi Bill,
>
> I realize that we are somewhat late in the cycle, but I think that this
> should be merged into the 3.5 branch.  Currently, an invocation of __umodsi3
> in a compiler-rt built with clang 3.5 for an ARM chip with idiv may fault
> (if you are lucky) or plain corrupt memory.
>
> The change is limited to ARMv7 with idiv and ARMv8 chips.
>
> On Sat, Aug 9, 2014 at 1:17 PM, Saleem Abdulrasool <compnerd at compnerd.org>
> wrote:
>>
>> Author: compnerd
>> Date: Sat Aug  9 15:17:37 2014
>> New Revision: 215295
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=215295&view=rev
>> Log:
>> builtins: correct __umodsi3, __udivsi3 on ARM
>>
>> When building the builtins for a modern CPU (idiv support), __umodsi3 was
>> completely incorrect as it would behave as __udivmosi3, which takes a
>> tertiary
>> parameter which is a pointer.
>>
>> __udivsi3 was also incorrect, returning the remainder in r1.  Although
>> this
>> would not result in any crash or invalid behaviour as r1 is a caller saved
>> register in AAPCS, this is unnecessary.  Simply perform the division
>> ignoring
>>
>> Modified:
>>     compiler-rt/trunk/lib/builtins/arm/udivsi3.S
>>     compiler-rt/trunk/lib/builtins/arm/umodsi3.S
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/udivsi3.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/udivsi3.S?rev=215295&r1=215294&r2=215295&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/udivsi3.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/udivsi3.S Sat Aug  9 15:17:37 2014
>> @@ -27,9 +27,7 @@ DEFINE_COMPILERRT_FUNCTION(__udivsi3)
>>  #if __ARM_ARCH_EXT_IDIV__
>>         tst     r1, r1
>>         beq     LOCAL_LABEL(divby0)
>> -       mov     r3, r0
>> -       udiv    r0, r3, r1
>> -       mls     r1, r0, r1, r3
>> +       udiv    r0, r0, r1
>>         bx      lr
>>  #else
>>         cmp     r1, #1
>>
>> Modified: compiler-rt/trunk/lib/builtins/arm/umodsi3.S
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/builtins/arm/umodsi3.S?rev=215295&r1=215294&r2=215295&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/builtins/arm/umodsi3.S (original)
>> +++ compiler-rt/trunk/lib/builtins/arm/umodsi3.S Sat Aug  9 15:17:37 2014
>> @@ -25,10 +25,8 @@ DEFINE_COMPILERRT_FUNCTION(__umodsi3)
>>  #if __ARM_ARCH_EXT_IDIV__
>>         tst     r1, r1
>>         beq     LOCAL_LABEL(divby0)
>> -       mov     r3, r0
>> -       udiv    r0, r3, r1
>> -       mls     r1, r0, r1, r3
>> -       str     r1, [r2]
>> +       udiv    r2, r0, r1
>> +       mls     r0, r2, r1, r0
>>         bx      lr
>>  #else
>>         cmp     r1, #1
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>