From patchwork Fri Jul 1 09:18:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrill Tkachov X-Patchwork-Id: 642885 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 3rgrSY624vz9sxR for ; Fri, 1 Jul 2016 19:18:52 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=LviRs60U; 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 :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=RGvUgbUNuOqA+GVppqpoM9W/tcAN5YYgLKWuHBqmi82Zz7 avaRXF9Iypk9g3/k04beKDGxPFSTUUCkloMJUeXd3uhtBumGzWo4cE1QAtyUHV5v UmsjyzKRIYHhl1GI8+g5JPBj+7OaF8KrNzsn49ipml7Q8DvnwaDZF4yAUUTQ4= 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 :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=ewrbLjWgj6LEMDXuhgN7ldB7cu4=; b=LviRs60UNKldoZFhBprC wYY7QKSaHTTAZR7mNk9TUaem1ADAYLdnIbRnMf8aJ8H5KW+CZbMgF5T66zTQylPk eqasT9ci+ys5rdMhD9ZnM204dD8fzKcTLuyVqzQdt3HinvIhRXXfP44LL/laOJ6g Oz4uSBAp/FZv3+a4f0AelgU= Received: (qmail 27482 invoked by alias); 1 Jul 2016 09:18:31 -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 27382 invoked by uid 89); 1 Jul 2016 09:18:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= 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; Fri, 01 Jul 2016 09:18:16 +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 99ABE2F for ; Fri, 1 Jul 2016 02:19:09 -0700 (PDT) Received: from [10.2.206.43] (e100706-lin.cambridge.arm.com [10.2.206.43]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AE8FC3F246 for ; Fri, 1 Jul 2016 02:18:14 -0700 (PDT) Message-ID: <57763554.2030807@foss.arm.com> Date: Fri, 01 Jul 2016 10:18:12 +0100 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: GCC Patches Subject: [PATCH][expr.c] PR middle-end/71700: zero-extend sub-word value when widening constructor element Hi all, In this arm wrong-code PR the struct assignment goes wrong when expanding constructor elements to a register destination when the constructor elements are signed bitfields less than a word wide. In this testcase we're intialising a struct with a 16-bit signed bitfield to -1 followed by a 1-bit bitfield to 0. Before it starts storing the elements it zeroes out the register. The code in store_constructor extends the first field to word size because it appears at the beginning of a word. It sign-extends the -1 to word size. However, when it later tries to store the 0 to bitposition 16 it has some logic to avoid redundant zeroing since the destination was originally cleared, so it doesn't emit the zero store. But the previous sign-extended -1 took up the whole word, so the position of the second bitfield contains a set bit. This patch fixes the problem by zeroing out the bits of the widened field that did not appear in the original value, so that we can safely avoid storing the second zero in the constructor. With this patch the testcase passes at all optimisation levels (it only passed at -O0 before). The initialisation of the struct to {-1, 0} now emits the RTL: (insn 5 2 6 2 (set (reg/v:SI 112 [ e ]) (const_int 0 [0])) (nil)) (insn 6 5 7 2 (set (reg/v:SI 112 [ e ]) (const_int 65535 [0xffff])) (nil)) whereas before it generated: (insn 5 4 6 (set (reg/v:SI 112 [ e ]) (const_int 0 [0])) (nil)) (insn 6 5 7 (set (reg/v:SI 112 [ e ]) (const_int -1 [0xffffffffffffffff])) (nil)) Bootstrapped and tested on arm, aarch64, x86_64 though the codepath is gated on WORD_REGISTER_OPERATIONS I didn't expect any effect on aarch64 and x86_64 anyway. Ok for trunk? This bug appears on all versions from 4.9 onwards so I'll be testing it on the branches as well. Thanks, Kyrill 2016-07-01 Kyrylo Tkachov PR middle-end/71700 * expr.c (store_constructor): Mask sign-extended bits when widening sub-word constructor element at the start of a word. 2016-07-01 Kyrylo Tkachov PR middle-end/71700 * gcc.c-torture/execute/pr71700.c: New test. diff --git a/gcc/expr.c b/gcc/expr.c index 9733401e09052457678a6a8817fe16f8a737927e..abb4416e41b60ab69f6f9d844e3a40853b0d1cde 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -6281,6 +6281,13 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size, type = lang_hooks.types.type_for_mode (word_mode, TYPE_UNSIGNED (type)); value = fold_convert (type, value); + /* Make sure the bits beyond the original bitsize are zero + so that we can correctly avoid extra zeroing stores in + later constructor elements. */ + tree bitsize_mask + = wide_int_to_tree (type, wi::mask (bitsize, false, + BITS_PER_WORD)); + value = fold_build2 (BIT_AND_EXPR, type, value, bitsize_mask); } if (BYTES_BIG_ENDIAN) diff --git a/gcc/testsuite/gcc.c-torture/execute/pr71700.c b/gcc/testsuite/gcc.c-torture/execute/pr71700.c new file mode 100644 index 0000000000000000000000000000000000000000..80afd3809c3abd24135fb36b757ace9f41ba3112 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr71700.c @@ -0,0 +1,19 @@ +struct S +{ + signed f0 : 16; + unsigned f1 : 1; +}; + +int b; +static struct S c[] = {{-1, 0}, {-1, 0}}; +struct S d; + +int +main () +{ + struct S e = c[0]; + d = e; + if (d.f1 != 0) + __builtin_abort (); + return 0; +}