From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29707 invoked by alias); 29 Oct 2015 11:32:20 -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 29680 invoked by uid 89); 29 Oct 2015 11:32:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yk0-f182.google.com Received: from mail-yk0-f182.google.com (HELO mail-yk0-f182.google.com) (209.85.160.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 29 Oct 2015 11:32:17 +0000 Received: by ykdr3 with SMTP id r3so38745646ykd.1 for ; Thu, 29 Oct 2015 04:32:15 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.129.70.65 with SMTP id t62mr832228ywa.240.1446118335584; Thu, 29 Oct 2015 04:32:15 -0700 (PDT) Received: by 10.37.117.136 with HTTP; Thu, 29 Oct 2015 04:32:15 -0700 (PDT) In-Reply-To: References: <20151014162944.GE16672@kam.mff.cuni.cz> <1456448.Tt4DpGVrAE@polaris> <20151021215701.GA14675@atrey.karlin.mff.cuni.cz> <2210014.Qgk3pAbeYD@polaris> <20151029033922.GA52478@kam.mff.cuni.cz> Date: Thu, 29 Oct 2015 11:32:00 -0000 Message-ID: Subject: Re: Add VIEW_CONVERT_EXPR to operand_equal_p From: Richard Biener To: Jan Hubicka Cc: Eric Botcazou , GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg03150.txt.bz2 On Thu, Oct 29, 2015 at 12:31 PM, Richard Biener wrote: > On Thu, Oct 29, 2015 at 12:22 PM, Richard Biener > wrote: >> On Thu, Oct 29, 2015 at 4:39 AM, Jan Hubicka wrote: >>>> > Added and comitted now. >>>> >>>> Thanks. Now on to the wrong code issues. :-) >>>> >>>> Up to the change, the useless_type_conversion_p predicate was relying on >>>> structural equivalence via the TYPE_CANONICAL check, now it only looks at the >>>> outermost level (size, mode). Now some back-ends, most notably x86-64, do a >>>> deep structural scan to determine the calling conventions (classify_argument) >>>> instead of just looking at the size and the mode, so consistency dictates that >>>> the type of the argument and that of the parameter be structurally equivalent >>>> and this sometimes can only be achieved by a VCE... which is now deleted. :-( >>>> See the call to derivedIP in the attached testcase which now fails on x86-64. >>>> >>>> How do we get away from here? >>> >>> Hmm, I noticed this in ipa-icf context and wrote checker that two functions are ABI >>> compatile (did not pushed it out yet), but of course this is nastier. >>> >>> I think the problem exists before my patch with LTO - it is just matter of >>> doing two types which will be considered equivalent by >>> gimple_canonical_types_compatible_p but have different type conversion. An >>> example of such type would be: >>> >>> struct a { >>> int a[4]; >>> }; >>> struct b { >>> int a[4]; >>> } __attribute__ ((__aligned__(16))); >>> >>> I tried to turn this into an testcase, the problem is that I don't know of a way >>> to obtain VIEW_CONVERT_EXPR between the two types out of C or C++ frontend and we >>> don't seem to synthetize these in middle end (even in cases it would make sense). >>> I will try to play with it more - would be nice to have a C reproducer. >>> >>> We may be safe before my patch from wrong code issues if there is no way to >>> rpduce VIEW_CONVERT_EXPR between types like this in languages that support >>> aligned attribute. >>> >>> I think the problem is generally similar to memory references - the gimple type >>> compatibility should not be tied to ABI details. Probably most consistent >>> solution would be to extend GIMPLE_CALL to also list types of parameters and do >>> not rely on whatever type the operand have.... >>> >>> Richard, any ideas? >> >> IMHO it was always wrong/fragile for backends to look at the actual arguments to >> decide on the calling convention. The backends should _solely_ rely on >> gimple_call_fntype and its TYPE_ARG_TYPES here. >> >> Of course then there are varargs ... (not sure if we hit this here). >> >> But yes, the VIEW_CONVERT "stripping" is a bit fragile and I don't remember >> what exactly we gain from it (when not done on registers). >> >> But I also don't see where we do the stripping mentioned on memory references. >> The match.pd pattern doesn't apply to memory, only in the GENERIC path >> which is guarded with exact type equality. So I can't see where we end up >> stripping the V_C_E. >> >> There is one bogus case still in fold-const.c: >> >> case VIEW_CONVERT_EXPR: >> if (TREE_CODE (op0) == MEM_REF) >> /* ??? Bogus for aligned types. */ >> return fold_build2_loc (loc, MEM_REF, type, >> TREE_OPERAND (op0, 0), TREE_OPERAND (op0, 1)); >> >> return NULL_TREE; >> >> that comment is only in my local tree ... (we lose alignment info that is >> on the original MEM_REF type which may be a smaller one). > > Ah - tree_ssa_useless_type_conversion and callers, during gimplification. > I'd like to get rid of it but maybe simply delete the VIEW_CONVERT_EXPR > case from it for now (and return true unconditionally for NON_LVALUE_EXPR). > > Index: gcc/tree-ssa.c > =================================================================== > --- gcc/tree-ssa.c (revision 229517) > +++ gcc/tree-ssa.c (working copy) > @@ -1142,13 +1161,16 @@ delete_tree_ssa (struct function *fn) > bool > tree_ssa_useless_type_conversion (tree expr) > { > + /* Not strictly a conversion but this function is used to strip > + useless stuff from trees returned from GENERIC folding. */ > + if (TREE_CODE (expr) == NON_LVALUE_EXPR) > + return true; > + > /* If we have an assignment that merely uses a NOP_EXPR to change > the top of the RHS to the type of the LHS and the type conversion > is "safe", then strip away the type conversion so that we can > enter LHS = RHS into the const_and_copies table. */ > - if (CONVERT_EXPR_P (expr) > - || TREE_CODE (expr) == VIEW_CONVERT_EXPR > - || TREE_CODE (expr) == NON_LVALUE_EXPR) > + if (CONVERT_EXPR_P (expr)) > return useless_type_conversion_p > (TREE_TYPE (expr), > TREE_TYPE (TREE_OPERAND (expr, 0))); > > IMHO the gimplifier use should be more explicit and most remaining GIMPLE > middle-end uses should be removed (after auditing). The above is pre-approved if one of you does the required testing (it fixes the Ada testcase for me). Richard. > Richard. > >> Richard. >> >>> Honza >>>> >>>> >>>> * gnat.dg/discr44.adb: New test. >>>> >>>> -- >>>> Eric Botcazou >>> >>>> -- { dg-do run } >>>> -- { dg-options "-gnatws" } >>>> >>>> procedure Discr44 is >>>> >>>> function Ident (I : Integer) return Integer is >>>> begin >>>> return I; >>>> end; >>>> >>>> type Int is range 1 .. 10; >>>> >>>> type Str is array (Int range <>) of Character; >>>> >>>> type Parent (D1, D2 : Int; B : Boolean) is record >>>> S : Str (D1 .. D2); >>>> end record; >>>> >>>> type Derived (D : Int) is new Parent (D1 => D, D2 => D, B => False); >>>> >>>> X1 : Derived (D => Int (Ident (7))); >>>> >>>> begin >>>> if X1.D /= 7 then >>>> raise Program_Error; >>>> end if; >>>> end; >>>