From patchwork Sun Dec 4 13:32:41 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kazu_Hirata@mentor.com X-Patchwork-Id: 129153 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 5297AB6F68 for ; Mon, 5 Dec 2011 00:33:02 +1100 (EST) Received: (qmail 22656 invoked by alias); 4 Dec 2011 13:33:00 -0000 Received: (qmail 22646 invoked by uid 22791); 4 Dec 2011 13:33:00 -0000 X-SWARE-Spam-Status: No, hits=3.3 required=5.0 tests=BAYES_00, MEDICAL_SUBJECT X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 04 Dec 2011 13:32:46 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1RXCBO-00009s-GI from Kazu_Hirata@mentor.com ; Sun, 04 Dec 2011 05:32:42 -0800 Received: from SVR-ORW-FEM-04.mgc.mentorg.com ([147.34.97.41]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Sun, 4 Dec 2011 05:32:33 -0800 Received: from build3-lucid-cs.sje.mentorg.com (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.1.289.1; Sun, 4 Dec 2011 05:32:41 -0800 Date: Sun, 4 Dec 2011 05:32:41 -0800 Message-ID: To: CC: , , , Subject: [patch] ARM: Fix miscompilation in arm.md:*minmax_arithsi. (PR target/51408) MIME-Version: 1.0 From: Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 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 PR target/51408 * gcc.dg/pr51408.c: New. Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 181985) +++ gcc/config/arm/arm.md (working copy) @@ -3413,7 +3413,7 @@ bool need_else; if (which_alternative != 0 || operands[3] != const0_rtx - || (code != PLUS && code != MINUS && code != IOR && code != XOR)) + || (code != PLUS && code != IOR && code != XOR)) need_else = true; else need_else = false; Index: gcc/testsuite/gcc.dg/pr51408.c =================================================================== --- gcc/testsuite/gcc.dg/pr51408.c (revision 0) +++ gcc/testsuite/gcc.dg/pr51408.c (revision 0) @@ -0,0 +1,22 @@ +/* This testcase used to fail because of a bug in + arm.md:*minmax_arithsi. */ + +/* { dg-do run } */ +/* { dg-options "-O1" } */ + +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; +}