From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30870 invoked by alias); 26 May 2011 11:24:54 -0000 Received: (qmail 30861 invoked by uid 22791); 26 May 2011 11:24:52 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST,TW_CF,TW_SV X-Spam-Check-By: sourceware.org Received: from mail-ww0-f51.google.com (HELO mail-ww0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 26 May 2011 11:24:37 +0000 Received: by wwf26 with SMTP id 26so552076wwf.8 for ; Thu, 26 May 2011 04:24:35 -0700 (PDT) MIME-Version: 1.0 Received: by 10.227.152.132 with SMTP id g4mr717708wbw.24.1306409075588; Thu, 26 May 2011 04:24:35 -0700 (PDT) Received: by 10.227.37.152 with HTTP; Thu, 26 May 2011 04:24:35 -0700 (PDT) In-Reply-To: References: <373566883.242226.1306404169706.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com> <1943095191.242428.1306405200685.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com> Date: Thu, 26 May 2011 12:05:00 -0000 Message-ID: Subject: Re: [patch gimplify]: Make sure comparison using boolean-type after gimplification From: Richard Guenther To: Kai Tietz Cc: Kai Tietz , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes 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 X-SW-Source: 2011-05/txt/msg02008.txt.bz2 On Thu, May 26, 2011 at 1:20 PM, Kai Tietz wrote: > 2011/5/26 Richard Guenther : >> On Thu, May 26, 2011 at 1:11 PM, Kai Tietz wro= te: >>> 2011/5/26 Richard Guenther : >>>> On Thu, May 26, 2011 at 12:46 PM, Richard Guenther >>>> wrote: >>>>> On Thu, May 26, 2011 at 12:20 PM, Kai Tietz wrote: >>>>>> Hello, >>>>>> >>>>>> this patch ensures that after gimplification also comparison express= ions using FE's boolean_type_node. =A0As we need to deal here with C/C++'s = (obj-c/c++ and java's), Ada's, and Fortran's specific boolean types, this p= atch alters some checks in tree-cfg for Ada's sake, and we need to deal in = fold-const about type-conversion of comparisons special. >>>>>> Additionally it takes care that in forwprop pass we don't do type ho= ising for boolean types. >>>>>> >>>>>> ChangeLog >>>>>> >>>>>> 2011-05-26 =A0Kai Tietz >>>>>> >>>>>> =A0 =A0 =A0 =A0 =A0* gimplify.c (gimple_boolify): Boolify all compar= ison >>>>>> =A0 =A0 =A0 =A0 =A0expressions. >>>>>> =A0 =A0 =A0 =A0 =A0(gimplify_expr): Use 'useless_type_conversion_p' = for comparing >>>>>> =A0 =A0 =A0 =A0 =A0org_type with boolean_type_node for TRUTH-express= ions and comparisons. >>>>>> =A0 =A0 =A0 =A0 =A0* fold-const.c (fold_unary_loc): Handle compariso= n conversions with >>>>>> =A0 =A0 =A0 =A0 =A0boolean-type special. >>>>>> =A0 =A0 =A0 =A0 =A0* tree-cfg.c (verify_gimple_comparison): Adjust c= heck for boolean >>>>>> =A0 =A0 =A0 =A0 =A0or compatible types. >>>>>> =A0 =A0 =A0 =A0 =A0(verify_gimple_assign_unary): Likewise. >>>>>> =A0 =A0 =A0 =A0 =A0* tree-ssa-forwprop.c (forward_propagate_comparis= on): Handle >>>>>> =A0 =A0 =A0 =A0 =A0boolean case special. >>>>>> >>>>>> Tested on x86_64-pc-linux-gnu (multilib) with regression test for al= l standard languages (C, C++, Obj-C, Fortran, Java) plus Obj-C++ and Ada. O= k for apply? >>>>> >>>>> It obviously isn't ok to apply before a patch has gone in that will r= esolve >>>>> all of the FAILs you specify. =A0Comments on the patch: >>>>> >>>>> @@ -7281,9 +7284,28 @@ gimplify_expr (tree *expr_p, gimple_seq >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 plain wrong if bitfields are involved= . =A0*/ >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0{ >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tree type =3D TREE_TYPE (TREE_OPER= AND (*expr_p, 1)); >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tree org_type =3D TREE_TYPE (*expr_= p); >>>>> + >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!useless_type_conversion_p (org= _type, boolean_type_node)) >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 TREE_TYPE (*expr_p) =3D boo= lean_type_node; >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *expr_p =3D fold_convert_lo= c (saved_location, org_type, *expr_p); >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D GS_OK; >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto dont_recalculate; >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >>>>> >>>>> The above should be only done for !AGGREGATE_TYPE_P. =A0Probably then >>>>> the strange dont_recalcuate goto can go away as well. >>>>> >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!AGGREGATE_TYPE_P (type)) >>>>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto expr_2; >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 enum gimplify_status r0, r1; >>>>> + >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 r0 =3D gimplify_expr (&TREE= _OPERAND (*expr_p, 0), pre_p, >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 post_p, is_gimple_val, fb_rvalue); >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 r1 =3D gimplify_expr (&TREE= _OPERAND (*expr_p, 1), pre_p, >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 post_p, is_gimple_val, fb_rvalue); >>>>> + >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D MIN (r0, r1); >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >>>>> + >>>>> >>>>> why change this? >>>>> >>>>> @@ -7641,6 +7641,12 @@ fold_unary_loc (location_t loc, enum tre >>>>> =A0 =A0 =A0 =A0} >>>>> =A0 =A0 =A0 else if (COMPARISON_CLASS_P (arg0)) >>>>> =A0 =A0 =A0 =A0{ >>>>> + =A0 =A0 =A0 =A0 /* Don't optimize type change, if op0 is of kind bo= olean_type_node. >>>>> + =A0 =A0 =A0 =A0 =A0 =A0Otherwise this will lead to race-condition o= n gimplification >>>>> + =A0 =A0 =A0 =A0 =A0 =A0trying to boolify comparison expression. =A0= */ >>>>> + =A0 =A0 =A0 =A0 if (TREE_TYPE (op0) =3D=3D boolean_type_node) >>>>> + =A0 =A0 =A0 =A0 =A0 return NULL_TREE; >>>>> + >>>>> =A0 =A0 =A0 =A0 =A0if (TREE_CODE (type) =3D=3D BOOLEAN_TYPE) >>>>> =A0 =A0 =A0 =A0 =A0 =A0{ >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0arg0 =3D copy_node (arg0); >>>>> >>>>> The code leading here looks quite strange to me ... >>>>> >>>>> tree >>>>> fold_unary_loc (location_t loc, enum tree_code code, tree type, tree = op0) >>>>> { >>>>> ... >>>>> =A0if (TREE_CODE_CLASS (code) =3D=3D tcc_unary) >>>>> =A0 =A0{ >>>>> ... >>>>> =A0 =A0 =A0else if (COMPARISON_CLASS_P (arg0)) >>>>> =A0 =A0 =A0 =A0{ >>>>> =A0 =A0 =A0 =A0 =A0if (TREE_CODE (type) =3D=3D BOOLEAN_TYPE) >>>>> =A0 =A0 =A0 =A0 =A0 =A0{ >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0arg0 =3D copy_node (arg0); >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0TREE_TYPE (arg0) =3D type; >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0return arg0; >>>>> =A0 =A0 =A0 =A0 =A0 =A0} >>>>> =A0 =A0 =A0 =A0 =A0else if (TREE_CODE (type) !=3D INTEGER_TYPE) >>>>> =A0 =A0 =A0 =A0 =A0 =A0return fold_build3_loc (loc, COND_EXPR, type, = arg0, >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fold_b= uild1_loc (loc, code, type, >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 integer_one_node), >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fold_b= uild1_loc (loc, code, type, >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 integer_zero_node)); >>>>> =A0 =A0 =A0 =A0} >>>>> >>>>> so, for any tcc_unary, like NEGATE_EXPR, with BOOLEAN_TYPE, >>>>> return arg0 ... sure. =A0Same for the 2nd case. =A0~ (a =3D=3D b) isn= 't >>>>> the same as a =3D=3D b ? ~1 : ~0. =A0I _suppose_ those cases were >>>>> ment for CONVERT_EXPR_CODE_P (code) instead of all of tcc_unary, >>>>> in which case they should be dropped or moved below where we >>>>> handle conversions explicitly. >>>>> >>>>> That said - does anyone remember anything about that above code? >>>>> Trying to do some svn blame history tracking now ... >>>> >>>> Oh, the patch continues... >>>> >>>> @@ -3208,7 +3208,10 @@ verify_gimple_comparison (tree type, tre >>>> =A0 =A0 =A0 =A0&& (!POINTER_TYPE_P (op0_type) >>>> =A0 =A0 =A0 =A0 =A0 || !POINTER_TYPE_P (op1_type) >>>> =A0 =A0 =A0 =A0 =A0 || TYPE_MODE (op0_type) !=3D TYPE_MODE (op1_type))) >>>> - =A0 =A0 =A0|| !INTEGRAL_TYPE_P (type)) >>>> + =A0 =A0 =A0|| !(TREE_CODE (type) =3D=3D BOOLEAN_TYPE >>>> + =A0 =A0 =A0 =A0 =A0|| (TREE_TYPE (type) && TREE_CODE (type) =3D=3D I= NTEGER_TYPE >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0&& TREE_CODE (TREE_TYPE (type)) =3D=3D BO= OLEAN_TYPE) >>>> + =A0 =A0 =A0 =A0 =A0|| (INTEGRAL_TYPE_P (type) && TYPE_PRECISION (typ= e) =3D=3D 1))) >>>> =A0 =A0 { >>>> >>>> why that strange TREE_TYPE (TREE_TYPE ())) thing again? =A0Drop >>>> that. >>>> >>>> @@ -3352,6 +3355,8 @@ verify_gimple_assign_unary (gimple stmt) >>>> =A0 =A0 case TRUTH_NOT_EXPR: >>>> =A0 =A0 =A0 /* We require two-valued operand types. =A0*/ >>>> =A0 =A0 =A0 if (!(TREE_CODE (rhs1_type) =3D=3D BOOLEAN_TYPE >>>> + =A0 =A0 =A0 =A0 =A0 || (TREE_TYPE (rhs1_type) && TREE_CODE (rhs1_typ= e) =3D=3D INTEGER_TYPE >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 && TREE_CODE (TREE_TYPE (rhs1_type)) =3D= =3D BOOLEAN_TYPE) >>>> =A0 =A0 =A0 =A0 =A0 =A0|| (INTEGRAL_TYPE_P (rhs1_type) >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&& TYPE_PRECISION (rhs1_type) =3D=3D 1)= )) >>>> =A0 =A0 =A0 =A0 { >>>> >>>> likewise. >>> >>> Well, those checks are necessary for Ada and its crippled >>> boolean_type_node and computed boolean-based integer construct. =A0Ada >>> derives here the boolean-type to an integer with range 0..1 and the >>> only way to find out that it is in fact such a beast is by looking >>> into TREE_TYPE of type. =A0See here Ada's code for getting base-type >>> information. >>> As such things are treated as compatible they can appear for TRUTH_NOT >>> expressions and comparisons. >> >> No they can't as we boolify them. >> >> Richard. > > Well, they do. =A0Test can prove this by running fortran's dg tests. > > The logic we do in gimplification is: > > 1) save original type of expression. > 2) set expression's type to boolean_type_node. > 3) if conversion from boolean_type_node to saved type isn't useless > (and here we have the issue about Fortran's different booleans with > different modes, which make those kinds not useless) then cast > expression's result to original type and retry gimplification. > 4) Otherwise gimplify operands and continue. You don't make sense. Btw, >>>> + if (!useless_type_conversion_p (org_type, boolean_ty= pe_node)) >>>> + { >>>> + TREE_TYPE (*expr_p) =3D boolean_type_node; >>>> + *expr_p =3D fold_convert_loc (saved_location, or= g_type, is bogus it should be TREE_TYPE (*expr_p) =3D boolean_type_node; if (!useless_type_conversion_p (org_type, boolean_type_node)) { *expr_p =3D fold_convert_loc (saved_location, org_type, ... > Regards, > Kai > > Kai >