From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 114280 invoked by alias); 15 Oct 2015 16:59:07 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 114265 invoked by uid 89); 15 Oct 2015 16:59:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,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; Thu, 15 Oct 2015 16:59:05 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 9A2665440FD; Thu, 15 Oct 2015 18:59:01 +0200 (CEST) Date: Thu, 15 Oct 2015 16:59:00 -0000 From: Jan Hubicka To: Richard Biener Cc: Jan Hubicka , Eric Botcazou , GCC Patches Subject: Re: Add VIEW_CONVERT_EXPR to operand_equal_p Message-ID: <20151015165900.GA64673@kam.mff.cuni.cz> References: <20151014162944.GE16672@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2015-10/txt/msg01497.txt.bz2 > > * fold-const.c (operand_equal_p): Handle VIEW_CONVERT_EXPR. > > Index: fold-const.c > > =================================================================== > > --- fold-const.c (revision 228735) > > +++ fold-const.c (working copy) > > @@ -2962,6 +2968,12 @@ operand_equal_p (const_tree arg0, const_ > > case IMAGPART_EXPR: > > return OP_SAME (0); > > > > + case VIEW_CONVERT_EXPR: > > + if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)) > > + && !types_compatible_p (TREE_TYPE (arg0), TREE_TYPE (arg1))) > > Note that mixing the GIMPLE types_compatible_p into operand_equal_p which > is supposed to handle GENERIC as well looks dangerous. You short-cutted > the type checks for OEP_ADDRESS_OF earlier so why do you need to > preserve them here? > > The test looks bogus anyway, but maybe it's just too early in the morning ;) > Shouldn't it be && types_compatible_p (...) (instead of && !types_com...)? > Otherwise it looks really weird. I think it is as intended: If we do !OEP_ADDRESS_OF we want to know that types are compatible. I am not quite sure this is needed - I wanted to mention that in an email, but it was too late in night for me :) In NOP_EXPR we check: /* Two conversions are equal only if signedness and modes match. */ switch (TREE_CODE (arg0)) { CASE_CONVERT: case FIX_TRUNC_EXPR: if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))) return 0; break; default: break; } And earlier we do: /* If both types don't have the same signedness, then we can't consider them equal. We must check this before the STRIP_NOPS calls because they may change the signedness of the arguments. As pointers strictly don't have a signedness, require either two pointers or two non-pointers as well. */ if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1)) || POINTER_TYPE_P (TREE_TYPE (arg0)) != POINTER_TYPE_P (TREE_TYPE (arg1))) return 0; So except for NOP_EXPR being stripped early this is bit redundant. I wondered what happens if I match two VCEs of very different type. Say vector of FLOATs and vector of INTEGERs and then convert them to same type that would result in differnt value. But this is not terribly special for VCE and probably would affect other expressions, too. I noticed now that this probably this can't happen because we also do: /* This is needed for conversions and for COMPONENT_REF. Might as well play it safe and always test this. */ if (TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1))) return 0; Which sounds somewhat conservative. At least should be skipped for OEP_ADDRESS_OF. You earlier mentioned BLKmode vectors. What would happen in this case? If I remember correctly. the check was not needed to pass testing, I am respawning testing without the type check. Honza > > Richard. > > > + return false; > > + return OP_SAME (0); > > + > > case TARGET_MEM_REF: > > case MEM_REF: > > if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)))