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.
