From patchwork Fri Nov 6 19:51:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 541095 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 ABD681402D4 for ; Sat, 7 Nov 2015 06:51:28 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=EnzvB+CT; 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:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=EhEEzi75GzmwDb0Fy o04ZbAN1CZIOI4OXiVvOqJXan3ADBSaFZvl9kBv4hhG0zD8oFkU1gQhCUZuf8cKw 12GGjC5LaT0WUct98xPyMO8Rmxu9RU98kNa8fqyfsNHtglL25xY5Hw9JmPs6wJBN zTlk449iOCbnXuVHXULg+IpsgQ= 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:references:mime-version :content-type:in-reply-to; s=default; bh=qPtPfidsRLGHTdiK5JJ/+KF 7D5Q=; b=EnzvB+CTF7lc4dJwazrbJcLqzMzqkiZEZQ6tE/afQlJfW+mvcCc1Vev sYs4hXKd5dRI3OLYKOb5B/1RqKQnRif5s7YvRE89Dwk7CpQ3wc12ML0za5j6RO6t SStNAp4PILCrD/u4wDeW4YBIjBi1xrD/OFiQjVMwxZMYvdy5DbhE= Received: (qmail 106970 invoked by alias); 6 Nov 2015 19:51:21 -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 106961 invoked by uid 89); 6 Nov 2015 19:51:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.1 required=5.0 tests=AWL, BAYES_40, KAM_ASCII_DIVIDERS, KAM_LAZY_DOMAIN_SECURITY, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 06 Nov 2015 19:51:18 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 5E858543950; Fri, 6 Nov 2015 20:51:14 +0100 (CET) Date: Fri, 6 Nov 2015 20:51:14 +0100 From: Jan Hubicka To: Richard Biener Cc: Jan Hubicka , gcc-patches@gcc.gnu.org Subject: Re: Allow matching of side effects in operand_equal_p Message-ID: <20151106195114.GB27859@kam.mff.cuni.cz> References: <20151102034853.GA91328@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) > On Mon, 2 Nov 2015, Jan Hubicka wrote: > > > Hi, > > this patch adds OEP_MATCH_SIDE_EFFECT to tell operand_equal_p that the two > > operands compared are from different code paths and thus they can be matched > > even if they have side effects. > > > > I.e. > > > > volatile int a; > > > > if (a==a) > > > > has two reads and thus can't be optimized (I guess), while > > > > b?a:a > > > > can be optimized. OEP_MATCH_SIDE_EFFECTS will be needed by ipa-icf. I changed > > gimple_operand_equal_value_p in tail-merge to use it. I do not know how to > > make the folding of ?: happen, since there seems to be no way to specify flags > > to match_operand in match.pd. This makes it hard to construct a testcase - all > > my attempts to trick tree-ssa-tailmerge to merge something that involve volatile > > operands failed, because it uses GVN to compare and GVN punt on these. > > > > I also wonder if we can drop OEP_CONSTANT_ADDRESS_OF. The only difference is that > > OEP_MATCH_SIDE_EFFECTS permits function calls. I think those should not appear > > in constant addresses. > > But you don't even need the call case for tailmerge or ICF as we don't > have CALL_EXPRs in GIMPLE. No? > > And you mean the difference of OEP_CONSTANT_ADDRESS_OF to > OEP_ADDRESS_OF | OEP_MATCH_SIDE_EFFECTS then, right? > > Ok if you go forward with that and do not change the CALL_EXPR case > for now (you may add a FIXME comment). Thank, this is variant I comitted which drops OEP_CONSTANT_ADDRESS_OF. Now I finally get what OEP_CONSTANT_ADDRESS_OF is for: we want to compare &volatilve_var == &volatile_var Obvoiusly when doing OEP_ADDRESS_OF, the TREE_SIDE_EFFECTS flag is not really informative in general. I suppose we could match &volatile_array[b] == &volatile_array[b] even if the address is no longer constant. Perhaps it is afe to ignore TREE_SIDE_EFFECTS for OEP_ADDRESS_OF in general? I can not see how address calculation can trigger side effects in other way than computing the offset (ie. volatile_var[func()]) but then the offset calculation is a normal expression and we do not get OEP_ADDRESS_OF. Honza * tree-core.h (size_type_kind): Remove OEP_CONSTANT_ADDRESS_OF and add OEP_MATCH_SIDE_EFFECTS. * fold-const.c (operand_equal_p): Update documentation; handle OEP_MATCH_SIDE_EFFECTS. * tree-ssa-tail-merge.c (gimple_operand_equal_value_p): Use OEP_MATCH_SIDE_EFFECTS. Index: tree-core.h =================================================================== --- tree-core.h (revision 229858) +++ tree-core.h (working copy) @@ -737,7 +737,7 @@ enum size_type_kind { enum operand_equal_flag { OEP_ONLY_CONST = 1, OEP_PURE_SAME = 2, - OEP_CONSTANT_ADDRESS_OF = 4, + OEP_MATCH_SIDE_EFFECTS = 4, OEP_ADDRESS_OF = 8 }; Index: fold-const.c =================================================================== --- fold-const.c (revision 229858) +++ fold-const.c (working copy) @@ -2649,8 +2649,7 @@ combine_comparisons (location_t loc, } /* Return nonzero if two operands (typically of the same tree node) - are necessarily equal. If either argument has side-effects this - function returns zero. FLAGS modifies behavior as follows: + are necessarily equal. FLAGS modifies behavior as follows: If OEP_ONLY_CONST is set, only return nonzero for constants. This function tests whether the operands are indistinguishable; @@ -2675,9 +2674,14 @@ combine_comparisons (location_t loc, to ensure that global memory is unchanged in between. If OEP_ADDRESS_OF is set, we are actually comparing addresses of objects, - not values of expressions. OEP_CONSTANT_ADDRESS_OF in addition to - OEP_ADDRESS_OF is used for ADDR_EXPR with TREE_CONSTANT flag set and we - further ignore any side effects on SAVE_EXPRs then. */ + not values of expressions. + + Unless OEP_MATCH_SIDE_EFFECTS is set, the function returns false on + any operand with side effect. This is unnecesarily conservative in the + case we know that arg0 and arg1 are in disjoint code paths (such as in + ?: operator). In addition OEP_MATCH_SIDE_EFFECTS is used when comparing + addresses with TREE_CONSTANT flag set so we know that &var == &var + even if var is volatile. */ int operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) @@ -2698,9 +2702,8 @@ operand_equal_p (const_tree arg0, const_ if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST) { /* Address of INTEGER_CST is not defined; check that we did not forget - to drop the OEP_ADDRESS_OF/OEP_CONSTANT_ADDRESS_OF flags. */ - gcc_checking_assert (!(flags - & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF))); + to drop the OEP_ADDRESS_OF flags. */ + gcc_checking_assert (!(flags & OEP_ADDRESS_OF)); return tree_int_cst_equal (arg0, arg1); } @@ -2806,7 +2809,7 @@ operand_equal_p (const_tree arg0, const_ they are necessarily equal as well. */ if (arg0 == arg1 && ! (flags & OEP_ONLY_CONST) && (TREE_CODE (arg0) == SAVE_EXPR - || (flags & OEP_CONSTANT_ADDRESS_OF) + || (flags & OEP_MATCH_SIDE_EFFECTS) || (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1)))) return 1; @@ -2865,11 +2868,10 @@ operand_equal_p (const_tree arg0, const_ TREE_STRING_LENGTH (arg0))); case ADDR_EXPR: - gcc_checking_assert (!(flags - & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF))); + gcc_checking_assert (!(flags & OEP_ADDRESS_OF)); return operand_equal_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 0), flags | OEP_ADDRESS_OF - | OEP_CONSTANT_ADDRESS_OF); + | OEP_MATCH_SIDE_EFFECTS); case CONSTRUCTOR: /* In GIMPLE empty constructors are allowed in initializers of aggregates. */ @@ -2928,7 +2930,7 @@ operand_equal_p (const_tree arg0, const_ /* If either of the pointer (or reference) expressions we are dereferencing contain a side effect, these cannot be equal, but their addresses can be. */ - if ((flags & OEP_CONSTANT_ADDRESS_OF) == 0 + if ((flags & OEP_MATCH_SIDE_EFFECTS) == 0 && (TREE_SIDE_EFFECTS (arg0) || TREE_SIDE_EFFECTS (arg1))) return 0; @@ -2936,11 +2938,11 @@ operand_equal_p (const_tree arg0, const_ switch (TREE_CODE (arg0)) { case INDIRECT_REF: - if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)) + if (!(flags & OEP_ADDRESS_OF) && (TYPE_ALIGN (TREE_TYPE (arg0)) != TYPE_ALIGN (TREE_TYPE (arg1)))) return 0; - flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF); + flags &= ~OEP_ADDRESS_OF; return OP_SAME (0); case REALPART_EXPR: @@ -2950,7 +2952,7 @@ operand_equal_p (const_tree arg0, const_ case TARGET_MEM_REF: case MEM_REF: - if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF))) + if (!(flags & OEP_ADDRESS_OF)) { /* Require equal access sizes */ if (TYPE_SIZE (TREE_TYPE (arg0)) != TYPE_SIZE (TREE_TYPE (arg1)) @@ -2975,7 +2977,7 @@ operand_equal_p (const_tree arg0, const_ != TYPE_ALIGN (TREE_TYPE (arg1))) return 0; } - flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF); + flags &= ~OEP_ADDRESS_OF; return (OP_SAME (0) && OP_SAME (1) /* TARGET_MEM_REF require equal extra operands. */ && (TREE_CODE (arg0) != TARGET_MEM_REF @@ -2990,7 +2992,7 @@ operand_equal_p (const_tree arg0, const_ may have different types but same value here. */ if (!OP_SAME (0)) return 0; - flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF); + flags &= ~OEP_ADDRESS_OF; return ((tree_int_cst_equal (TREE_OPERAND (arg0, 1), TREE_OPERAND (arg1, 1)) || OP_SAME (1)) @@ -3003,13 +3005,13 @@ operand_equal_p (const_tree arg0, const_ if (!OP_SAME_WITH_NULL (0) || !OP_SAME (1)) return 0; - flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF); + flags &= ~OEP_ADDRESS_OF; return OP_SAME_WITH_NULL (2); case BIT_FIELD_REF: if (!OP_SAME (0)) return 0; - flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF); + flags &= ~OEP_ADDRESS_OF; return OP_SAME (1) && OP_SAME (2); default: @@ -3021,9 +3023,7 @@ operand_equal_p (const_tree arg0, const_ { case ADDR_EXPR: /* Be sure we pass right ADDRESS_OF flag. */ - gcc_checking_assert (!(flags - & (OEP_ADDRESS_OF - | OEP_CONSTANT_ADDRESS_OF))); + gcc_checking_assert (!(flags & OEP_ADDRESS_OF)); return operand_equal_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 0), flags | OEP_ADDRESS_OF); @@ -3089,6 +3089,7 @@ operand_equal_p (const_tree arg0, const_ return 0; } + /* FIXME: We could skip this test for OEP_MATCH_SIDE_EFFECTS. */ { unsigned int cef = call_expr_flags (arg0); if (flags & OEP_PURE_SAME) Index: tree-ssa-tail-merge.c =================================================================== --- tree-ssa-tail-merge.c (revision 229858) +++ tree-ssa-tail-merge.c (working copy) @@ -1081,7 +1081,7 @@ gimple_operand_equal_value_p (tree t1, t || t2 == NULL_TREE) return false; - if (operand_equal_p (t1, t2, 0)) + if (operand_equal_p (t1, t2, OEP_MATCH_SIDE_EFFECTS)) return true; return gvn_uses_equal (t1, t2);