[PATCH] EABI Divmod, take #1

[Renato Golin]

On 11 July 2013 10:41, Kristof Beyls <kristof.beyls at arm.com> wrote:

> 1. Would it be possible to also add checks that check if the values
>
>    are returned in r0,r1 in EABI mode? I'm not sure if it would be****
>
>    a good idea, but e.g. when compiling at -O2, I would expect for****
>
>    the testcase in function f, it could be checked that the****
>
>    instruction right after __aeabi_idivmod adds r0 and r1?
>

Yes! I thought I had done that but only realized that I hadn't when
preparing the patch for review. I'll do that before commit.



> ****
>
>    Or is that not possible at the moment, because currently 2 calls****
>
>    to __aeabi_idivmod are being generated instead of 1 call for ****
>
>    (a/b,a%b)?
>

No, that doesn't change anything, but it is an inefficiency that I haven't
investigated. Will report it on bugzilla.



> ****
>
> ** 2. It seems the regression tests only check the behaviour for i32 and
>
>    i64 data types. Would it be a good idea to adapt some of the current***
> *
>
>    tests to instead check i8 and i16 data types, so all data types are****
>
>    covered in the tests?
>

The tests for i8 and i16 are redundant, because the behaviour is identical
for i8, i16 and i32, but I can add them in the tests. I have them locally,
but didn't put in the test.


****
>
> ** 3. The check in functions f, g1 and g4 look very similar. Maybe only
> one
>
>    of these functions need to be retained? I can't identify what testing**
> **
>
>    value g1 and f have over g4.
>
They're slightly different, as I was trying to see if the returned values
were being associated correctly on the correct registers on all patterns.
f32 is the original code, testing 32-bit values and multiple uses. g1 is a
simple usage of divmod, and g4 uses the result of div to calculate the rem.

cheers,
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130711/6e6e8f65/attachment.html>