On 05/12/11 10:42, Richard Earnshaw wrote:
> On 04/12/11 13:32, kazu_hir...@mentor.com wrote:
>> Hi,
>>
>> Attached is a patch to fix miscompilation in
>> arm.md:*minmax_arithsi.
>>
>> The following testcase, reduced from efgcvt_r.c:fcvt_r in glibc, gets
>> miscompiled:
>>
>> extern void abort (void);
>>
>> int __attribute__((noinline))
>> foo (int a, int b)
>> {
>> int max = (b > 0) ? b : 0;
>> return max - a;
>> }
>>
>> int
>> main (void)
>> {
>> if (foo (3, -1) != -3)
>> abort ();
>> return 0;
>> }
>>
>> arm-none-eabi-gcc -O1 generates:
>>
>> foo:
>> @ Function supports interworking.
>> @ args = 0, pretend = 0, frame = 0
>> @ frame_needed = 0, uses_anonymous_args = 0
>> @ link register save eliminated.
>> cmp r1, #0
>> rsbge r0, r0, r1
>> bx lr
>>
>> This would be equivalent to:
>>
>> return b >= 0 ? b - a : a;
>>
>> which is different from:
>>
>> return b >= 0 ? b - a : -a;
>>
>> That is, in assembly code, we should have an "else" clause like so:
>>
>> cmp r1, #0
>> rsbge r0, r0, r1 <- then clause
>> rsblt r0, r0, #0 <- else clause
>> bx lr
>>
>> The problem comes from the fact that arm.md:*minmax_arithsi does not
>> add the "else" clause even though MINUS is not commutative.
>>
>> The patch fixes the problem by always requiring the "else" clause in
>> the MINUS case.
>>
>> Tested by running gcc testsuite on various ARM subarchitectures. OK
>> to apply?
>>
>> Kazu Hirata
>>
>> gcc/
>> 2011-12-04 Kazu Hirata <k...@codesourcery.com>
>>
>> PR target/51408
>> * config/arm/arm.md (*minmax_arithsi): Always require the else
>> clause in the MINUS case.
>>
>> gcc/testsuite/
>> 2011-12-04 Kazu Hirata <k...@codesourcery.com>
>>
>> PR target/51408
>> * gcc.dg/pr51408.c: New.
>>
>
> OK.
>
> R.
BTW, I would expect this to also exist in all the release branches. Could you
back-port it where
needed please.
TIA,
R.
