From patchwork Tue Jan 5 05:53:37 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Preudhomme X-Patchwork-Id: 562951 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 51A53140324 for ; Tue, 5 Jan 2016 16:53:54 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=rWg2YnFT; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version :content-transfer-encoding:content-type; q=dns; s=default; b=csd MwRu0bsK+l2mE4cAui1iZyI1meE908HGngZcmsQ/eZL4pgAaVZ13CDZUrPltSgb9 FXk8DDc0lHbDE4VeHqEyflghArPSM6Aa7togOV7s81AbQ+7dPWllYK/UxlE6+WNQ 8O9lc5EjL4CJ+oyHhMOU98Rza4zSzxzIHUubq0rA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version :content-transfer-encoding:content-type; s=default; bh=jZT8gp1xl g1LCpoysMoJ3MvdJSo=; b=rWg2YnFTMPqqrIlT9Hdoxr1RkYLz4oPQTiwI3s6LA oEPwAfNllROUYUNPHped4gmPyGCFtug8lxy/tux+X/dtuaBQWCduwqKJ2d3d17V0 Tf6Iw0kh9oHXMuTswWxtuQhfVkbfT8evhR5mvPTkC5TwXTJX+sfU7EjOUjzePmeU qU= Received: (qmail 59614 invoked by alias); 5 Jan 2016 05:53:46 -0000 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 Received: (qmail 59568 invoked by uid 89); 5 Jan 2016 05:53:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=Part, __char_bit__, __CHAR_BIT__, uint32_t X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 05 Jan 2016 05:53:44 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8A37A49; Mon, 4 Jan 2016 21:53:10 -0800 (PST) Received: from hardin.shanghai.arm.com (thomas-desktop.shanghai.arm.com [10.164.2.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7DC073F246; Mon, 4 Jan 2016 21:53:42 -0800 (PST) From: Thomas Preud'homme To: gcc-patches@gcc.gnu.org, Richard Biener Subject: [PATCH, GCC] Fix PR67781: wrong code generation for partial load on big endian targets Date: Tue, 05 Jan 2016 13:53:37 +0800 Message-ID: <5842907.YBOs7q9Hb0@hardin.shanghai.arm.com> User-Agent: KMail/4.14.7 (Linux/3.19.0-42-generic; KDE/4.14.8; x86_64; ; ) MIME-Version: 1.0 Hi, bswap optimization pass generate wrong code on big endian targets when the result of a bit operation it analyzed is a partial load of the range of memory accessed by the original expression (when one or more bytes at lowest address were lost in the computation). This is due to the way cmpxchg and cmpnop are adjusted in find_bswap_or_nop before being compared to the result of the symbolic expression. Part of the adjustment is endian independent: it's to ignore the bytes that were not accessed by the original gimple expression. However, when the result has less byte than that original expression, some more byte need to be ignored and this is endian dependent. The current code only support loss of bytes at the highest addresses because there is no code to adjust the address of the load. However, for little and big endian targets the bytes at highest address translate into different byte significance in the result. This patch first separate cmpxchg and cmpnop adjustement into 2 steps and then deal with endianness correctly for the second step. ChangeLog entries are as follow: *** gcc/ChangeLog *** 2015-12-16 Thomas Preud'homme PR tree-optimization/67781 * tree-ssa-math-opts.c (find_bswap_or_nop): Zero out bytes in cmpxchg and cmpnop in two steps: first the ones not accessed in original gimple expression in a endian independent way and then the ones not accessed in the final result in an endian-specific way. *** gcc/testsuite/ChangeLog *** 2015-12-16 Thomas Preud'homme PR tree-optimization/67781 * gcc.c-torture/execute/pr67781.c: New file. according to the size of the symbolic number before using it. */ @@ -2464,24 +2466,38 @@ find_bswap_or_nop (gimple *stmt, struct symbolic_number *n, bool *bswap) /* Find real size of result (highest non-zero byte). */ if (n->base_addr) - { - int rsize; - uint64_t tmpn; - - for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER, rsize++); - n->range = rsize; - } + for (tmpn = n->n, rsize = 0; tmpn; tmpn >>= BITS_PER_MARKER, rsize++); + else + rsize = n->range; - /* Zero out the extra bits of N and CMP*. */ + /* Zero out the bits corresponding to untouched bytes in original gimple + expression. */ if (n->range < (int) sizeof (int64_t)) { - uint64_t mask; - mask = ((uint64_t) 1 << (n->range * BITS_PER_MARKER)) - 1; cmpxchg >>= (64 / BITS_PER_MARKER - n->range) * BITS_PER_MARKER; cmpnop &= mask; } + /* Zero out the bits corresponding to unused bytes in the result of the + gimple expression. */ + if (rsize < n->range) + { + if (BYTES_BIG_ENDIAN) + { + mask = ((uint64_t) 1 << (rsize * BITS_PER_MARKER)) - 1; + cmpxchg &= mask; + cmpnop >>= (n->range - rsize) * BITS_PER_MARKER; + } + else + { + mask = ((uint64_t) 1 << (rsize * BITS_PER_MARKER)) - 1; + cmpxchg >>= (n->range - rsize) * BITS_PER_MARKER; + cmpnop &= mask; + } + n->range = rsize; + } + /* A complete byte swap should make the symbolic number to start with the largest digit in the highest order byte. Unchanged symbolic number indicates a read with same endianness as target architecture. */ Regression testsuite was run on a bootstrapped native x86_64-linux-gnu GCC and on an arm-none-eabi GCC cross-compiler without any regression. I'm waiting for a slot on gcc110 to do a big endian bootstrap but at least the testcase works on mips-linux. I'll send an update once bootstrap is complete. Is this ok for trunk and 5 branch in a week time if no regression is reported? Best regards, Thomas diff --git a/gcc/testsuite/gcc.c-torture/execute/pr67781.c b/gcc/testsuite/gcc.c-torture/execute/pr67781.c new file mode 100644 index 0000000..bf50aa2 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr67781.c @@ -0,0 +1,34 @@ +#ifdef __UINT32_TYPE__ +typedef __UINT32_TYPE__ uint32_t; +#else +typedef unsigned uint32_t; +#endif + +#ifdef __UINT8_TYPE__ +typedef __UINT8_TYPE__ uint8_t; +#else +typedef unsigned char uint8_t; +#endif + +struct +{ + uint32_t a; + uint8_t b; +} s = { 0x123456, 0x78 }; + +int pr67781() +{ + uint32_t c = (s.a << 8) | s.b; + return c; +} + +int +main () +{ + if (sizeof (uint32_t) * __CHAR_BIT__ != 32) + return 0; + + if (pr67781 () != 0x12345678) + __builtin_abort (); + return 0; +} diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index b00f046..e5a185f 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -2441,6 +2441,8 @@ find_bswap_or_nop_1 (gimple *stmt, struct symbolic_number *n, int limit) static gimple * find_bswap_or_nop (gimple *stmt, struct symbolic_number *n, bool *bswap) { + unsigned rsize; + uint64_t tmpn, mask; /* The number which the find_bswap_or_nop_1 result should match in order to have a full byte swap. The number is shifted to the right