From patchwork Fri Nov 10 13:51:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 836750 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-466503-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="KTfyT3pa"; dkim-atps=neutral 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 3yYM0j0phhz9sxR for ; Sat, 11 Nov 2017 00:52:19 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; q=dns; s=default; b=JfJaMJ0iZ4LvKOuLjfyyjcsz1hbjZ +OARyWmKZcz82PTMq7neVQ+FokMB5s36H9M42V8Qt3Y1yR1JcdiQ+Hllli1gtme6 FLJM69Yl3sgav3FGL7z9ntKCx8PBfJ/XEcxqxTsTh/UAO8Vowx4bqn5tPlds4PzQ 5DNC2XCtqhoWAE= 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:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; s=default; bh=epyphkXQyCq89h0vw5aMqYtiybo=; b=KTf yT3pag5tvuCuRYo5azqTERCR6JLJ9dMr/UbSs6VlDqdPPgZ2A88JMXSorIpdMadN i1MKXg8QN3gspw4tHPoSFPWr7FvUpM3vbhhwwmboHWiN7hNiWvZN4byvFIxcLhpo aEU/eDMFmZNRb+I/ALieA49gd08OFqgJJeN+r6wM= Received: (qmail 69499 invoked by alias); 10 Nov 2017 13:52:08 -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 69009 invoked by uid 89); 10 Nov 2017 13:52:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 10 Nov 2017 13:52:06 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C955C5D9EE; Fri, 10 Nov 2017 13:52:04 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-247.ams2.redhat.com [10.36.116.247]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3691F6268D; Fri, 10 Nov 2017 13:52:04 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id vAADq1ol018924; Fri, 10 Nov 2017 14:52:02 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id vAADpxnf018923; Fri, 10 Nov 2017 14:51:59 +0100 Date: Fri, 10 Nov 2017 14:51:59 +0100 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix ICE in store-merging (PR tree-optimization/82929) Message-ID: <20171110135159.GL14653@tucnak> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes Hi! Because BIT_{AND,IOR,XOR}_EXPR are commutative, we std::swap the store_operand_info ops if that means we could append the store into a group rather than having to start a new group. Unfortunately for count_multiple_uses we need to walk the stmts computing the stored value in order to check the has_single_use stuff and if we've swapped the arguments, that confuses the function. This patch just records that we've swapped them and then the function can take that into account easily. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2017-11-10 Jakub Jelinek PR tree-optimization/82929 * gimple-ssa-store-merging.c (struct store_immediate_info): Add ops_swapped_p non-static data member. (store_immediate_info::store_immediate_info): Clear it. (imm_store_chain_info::coalesce_immediate_stores): If swapping ops set ops_swapped_p. (count_multiple_uses): Handle ops_swapped_p. * gcc.dg/pr82929.c: New test. * g++.dg/opt/pr82929.C: New test. Jakub --- gcc/gimple-ssa-store-merging.c.jj 2017-11-09 20:24:34.000000000 +0100 +++ gcc/gimple-ssa-store-merging.c 2017-11-10 08:04:49.192744048 +0100 @@ -209,7 +209,11 @@ struct store_immediate_info /* INTEGER_CST for constant stores, MEM_REF for memory copy or BIT_*_EXPR for logical bitwise operation. */ enum tree_code rhs_code; + /* True if BIT_{AND,IOR,XOR}_EXPR result is inverted before storing. */ bool bit_not_p; + /* True if ops have been swapped and thus ops[1] represents + rhs1 of BIT_{AND,IOR,XOR}_EXPR and ops[0] represents rhs2. */ + bool ops_swapped_p; /* Operands. For BIT_*_EXPR rhs_code both operands are used, otherwise just the first one. */ store_operand_info ops[2]; @@ -231,7 +235,8 @@ store_immediate_info::store_immediate_in const store_operand_info &op0r, const store_operand_info &op1r) : bitsize (bs), bitpos (bp), bitregion_start (brs), bitregion_end (bre), - stmt (st), order (ord), rhs_code (rhscode), bit_not_p (bitnotp) + stmt (st), order (ord), rhs_code (rhscode), bit_not_p (bitnotp), + ops_swapped_p (false) #if __cplusplus >= 201103L , ops { op0r, op1r } { @@ -1186,7 +1191,10 @@ imm_store_chain_info::coalesce_immediate == info->bitpos - infof->bitpos) && operand_equal_p (info->ops[1].base_addr, infof->ops[0].base_addr, 0)) - std::swap (info->ops[0], info->ops[1]); + { + std::swap (info->ops[0], info->ops[1]); + info->ops_swapped_p = true; + } if ((!infof->ops[0].base_addr || compatible_load_p (merged_store, info, base_addr, 0)) && (!infof->ops[1].base_addr @@ -1410,18 +1418,21 @@ count_multiple_uses (store_immediate_inf stmt = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (stmt)); /* stmt is now the BIT_*_EXPR. */ if (!has_single_use (gimple_assign_rhs1 (stmt))) - ret += 1 + info->ops[0].bit_not_p; - else if (info->ops[0].bit_not_p) + ret += 1 + info->ops[info->ops_swapped_p].bit_not_p; + else if (info->ops[info->ops_swapped_p].bit_not_p) { gimple *stmt2 = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (stmt)); if (!has_single_use (gimple_assign_rhs1 (stmt2))) ++ret; } if (info->ops[1].base_addr == NULL_TREE) - return ret; + { + gcc_checking_assert (!info->ops_swapped_p); + return ret; + } if (!has_single_use (gimple_assign_rhs2 (stmt))) - ret += 1 + info->ops[1].bit_not_p; - else if (info->ops[1].bit_not_p) + ret += 1 + info->ops[1 - info->ops_swapped_p].bit_not_p; + else if (info->ops[1 - info->ops_swapped_p].bit_not_p) { gimple *stmt2 = SSA_NAME_DEF_STMT (gimple_assign_rhs2 (stmt)); if (!has_single_use (gimple_assign_rhs1 (stmt2))) --- gcc/testsuite/gcc.dg/pr82929.c.jj 2017-11-10 08:10:14.399799845 +0100 +++ gcc/testsuite/gcc.dg/pr82929.c 2017-11-10 08:18:24.131904561 +0100 @@ -0,0 +1,18 @@ +/* PR tree-optimization/82929 */ +/* { dg-do compile { target store_merge } } */ +/* { dg-options "-O2 -fdump-tree-store-merging" } */ + +void +foo (short *p, short *q, short *r) +{ + short a = q[0]; + short b = q[1]; + short c = ~a; + short d = r[0]; + short e = r[1]; + short f = ~b; + p[0] = c & d; + p[1] = e & f; +} + +/* { dg-final { scan-tree-dump-times "Merging successful" 1 "store-merging" } } */ --- gcc/testsuite/g++.dg/opt/pr82929.C.jj 2017-11-10 08:17:35.843479732 +0100 +++ gcc/testsuite/g++.dg/opt/pr82929.C 2017-11-10 08:16:16.000000000 +0100 @@ -0,0 +1,30 @@ +// PR tree-optimization/82929 +// { dg-do compile } +// { dg-options "-O2" } + +template struct A { + long _M_w[_Nw]; + void m_fn1(A p1) { + for (int a = 0;; a++) + _M_w[a] &= p1._M_w[a]; + } + void m_fn2() { + for (int b = 0; b < _Nw; b++) + _M_w[b] = ~_M_w[b]; + } +}; +template struct C : A<_Nb / (8 * 8)> { + void operator&=(C p1) { this->m_fn1(p1); } + C m_fn3() { + this->m_fn2(); + return *this; + } + C operator~() { return C(*this).m_fn3(); } +}; +struct B { + C<192> Value; +}; +void fn1(C<192> &p1) { + B c; + p1 &= ~c.Value; +}