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

Jim Grosbach grosbach at apple.com
Tue Aug 19 15:55:46 PDT 2014


Thanks for the archaeology, folks. That answered my questions about how this came to be. LGTM for 3.5.
-j
> On Aug 19, 2014, at 3:40 PM, Renato Golin <renato.golin at linaro.org> wrote:
> 
> So, we have look and we believe that this time, it's correct. Saleem's
> patch does indeed fixes the two blatant bugs that we introduced by
> copy&paste.
> 
> Jim/Bill, we also believe this fix needs to be in 3.5, as things will
> be completely broken if not. Can I merge it back?
> 
> cheers,
> --renato
> 
> On 19 August 2014 23:04, Renato Golin <renato.golin at linaro.org> wrote:
>> On 19 August 2014 22:55, Anton Korobeynikov <anton at korobeynikov.info> wrote:
>>> Did some git / svn archaeology: support for cores with division
>>> instructions was added in r182665 / a09d09d29e
>>> 
>>> And the code there (see e.g.
>>> http://llvm.org/klaus/compiler-rt/commit/a09d09d29e250e905bdfaf819979b9c3e9adc047/)
>>> is precisely the same as in Saleem's latest commit.
>>> 
>>> Looks like in r200001 umodsi3.S was completely replaced (!) by
>>> udivmodsi4.S making it broken.
>>> 
>>> CC'ing Joerg as he was the author of that commit.
>> 
>> I think we need to look at this one carefully...
>> 
>> Joerg may correct me, but I believe the intention was to implement a
>> divmod, that happened to be in a badly named file (umod) and with a
>> copy&paste mistake on the function name. So that one should actually
>> be the divmod function, not the mod, and the latest Saleem's patch
>> actually finished killing the original implementation at all, meaning
>> that it's still divmod for armv4 but not for the rest, but being used
>> as div/mod.
>> 
>> I can't believe we used to pass all tests with compiler-rt on ARM
>> about two months ago with that all messed up!
>> 
>> This is the review in Phab:
>> 
>> http://reviews.llvm.org/D2595
>> 
>> cheers,
>> --renato




More information about the llvm-commits mailing list