This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] ARM: Fix miscompilation in arm.md:*minmax_arithsi. (PR target/51408)
- From: Richard Earnshaw <rearnsha at arm dot com>
- To: "Kazu_Hirata at mentor dot com" <Kazu_Hirata at mentor dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, "nickc at redhat dot com" <nickc at redhat dot com>, "paul at codesourcery dot com" <paul at codesourcery dot com>, Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>
- Date: Mon, 05 Dec 2011 10:42:17 +0000
- Subject: Re: [patch] ARM: Fix miscompilation in arm.md:*minmax_arithsi. (PR target/51408)
- References: <yxj5hb1gzcli.fsf@build3-lucid-cs.sje.mentorg.com>
On 04/12/11 13:32, Kazu_Hirata@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 <kazu@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 <kazu@codesourcery.com>
>
> PR target/51408
> * gcc.dg/pr51408.c: New.
>
OK.
R.