From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28104 invoked by alias); 10 May 2011 14:44:41 -0000 Received: (qmail 27944 invoked by uid 22791); 10 May 2011 14:44:39 -0000 X-SWARE-Spam-Status: No, hits=-0.4 required=5.0 tests=AWL,BAYES_80,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST X-Spam-Check-By: sourceware.org Received: from mail-ww0-f41.google.com (HELO mail-ww0-f41.google.com) (74.125.82.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 10 May 2011 14:44:23 +0000 Received: by wwi18 with SMTP id 18so3392937wwi.2 for ; Tue, 10 May 2011 07:44:21 -0700 (PDT) MIME-Version: 1.0 Received: by 10.227.98.9 with SMTP id o9mr4231992wbn.25.1305038661628; Tue, 10 May 2011 07:44:21 -0700 (PDT) Received: by 10.227.20.74 with HTTP; Tue, 10 May 2011 07:44:21 -0700 (PDT) In-Reply-To: References: Date: Tue, 10 May 2011 15:24:00 -0000 Message-ID: Subject: Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary From: Richard Guenther To: Kai Tietz Cc: GCC Patches 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/msg00749.txt.bz2 On Tue, May 10, 2011 at 3:56 PM, Kai Tietz wrote: > Hello, > > this patch converts TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR > expressions > on gimplification to their binary form. =A0Additionally it takes care > that conditions > are getting boolified for operation. > > ChangeLog > > 2011-05-10 =A0Kai Tietz > > =A0 =A0 =A0 =A0* gimplify.c (gimplify_exit_expr): Boolify conditional > =A0 =A0 =A0 =A0expression part. > =A0 =A0 =A0 =A0(shortcut_cond_r): Likewise. > =A0 =A0 =A0 =A0(shortcut_cond_expr): Likewise. > =A0 =A0 =A0 =A0(gimplify_cond_expr): Likewise. > =A0 =A0 =A0 =A0(gimplify_modify_expr_rhs): Likewise. > =A0 =A0 =A0 =A0(gimplify_boolean_expr): Likewise. > =A0 =A0 =A0 =A0(gimple_boolify): Boolify operands for BOOLEAN typed > =A0 =A0 =A0 =A0base expressions. > =A0 =A0 =A0 =A0(gimplify_expr): Boolify TRUTH_ANDIF_EXPR, TRUTH_ORIF_EXPR, > =A0 =A0 =A0 =A0TRUTH_AND_EXPR, TRUTH_OR_EXPR, and TRUTH_XOR_EXPR. Additio= nally > =A0 =A0 =A0 =A0move TRUTH_AND|OR|XOR_EXPR to its binary form. > > Tested for x86_64-w64-mingw32 and x86_64-pc-linux-gnu. Ok for apply? Comments inline (the attachment makes my comments harder to spot, look close). Index: gcc/gcc/gimplify.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- gcc.orig/gcc/gimplify.c 2011-05-10 15:44:49.000000000 +0200 +++ gcc/gcc/gimplify.c 2011-05-10 15:46:58.365473600 +0200 @@ -1664,10 +1664,12 @@ build_and_jump (tree *label_p) static enum gimplify_status gimplify_exit_expr (tree *expr_p) { - tree cond =3D TREE_OPERAND (*expr_p, 0); - tree expr; + tree cond, expr; + TREE_OPERAND (*expr_p, 0) =3D gimple_boolify (TREE_OPERAND (*expr_p, 0)); + cond =3D TREE_OPERAND (*expr_p, 0); Why do you need to boolify things at all when we build a COND_EXPR that gets re-gimplified anyway? I'm confused by this (similar to other places you do that). I would expect that you at most would do this when gimplifying COND_EXPRs, not when you build them. expr =3D build_and_jump (&gimplify_ctxp->exit_label); + expr =3D build3 (COND_EXPR, void_type_node, cond, expr, NULL_TREE); *expr_p =3D expr; @@ -2586,6 +2588,7 @@ shortcut_cond_r (tree pred, tree *true_l /* Keep the original source location on the first 'if'. Set the sou= rce location of the ? on the second 'if'. */ new_locus =3D EXPR_HAS_LOCATION (pred) ? EXPR_LOCATION (pred) : locu= s; + TREE_OPERAND (pred, 0) =3D gimple_boolify (TREE_OPERAND (pred, 0)); expr =3D build3 (COND_EXPR, void_type_node, TREE_OPERAND (pred, 0), shortcut_cond_r (TREE_OPERAND (pred, 1), true_label_p, false_label_p, locus), @@ -2594,7 +2597,7 @@ shortcut_cond_r (tree pred, tree *true_l } else { - expr =3D build3 (COND_EXPR, void_type_node, pred, + expr =3D build3 (COND_EXPR, void_type_node, gimple_boolify (pred), build_and_jump (true_label_p), build_and_jump (false_label_p)); SET_EXPR_LOCATION (expr, locus); @@ -2625,7 +2628,8 @@ shortcut_cond_expr (tree expr) bool emit_end, emit_false, jump_over_else; bool then_se =3D then_ && TREE_SIDE_EFFECTS (then_); bool else_se =3D else_ && TREE_SIDE_EFFECTS (else_); - + + pred =3D TREE_OPERAND (expr, 0) =3D gimple_boolify (TREE_OPERAND (expr, = 0)); /* First do simple transformations. */ if (!else_se) { @@ -2665,7 +2669,7 @@ shortcut_cond_expr (tree expr) SET_EXPR_LOCATION (expr, EXPR_LOCATION (pred)); else_ =3D shortcut_cond_expr (expr); else_se =3D else_ && TREE_SIDE_EFFECTS (else_); - pred =3D TREE_OPERAND (pred, 0); + pred =3D gimple_boolify (TREE_OPERAND (pred, 0)); expr =3D build3 (COND_EXPR, void_type_node, pred, NULL_TREE, else_); SET_EXPR_LOCATION (expr, locus); } @@ -2824,9 +2828,6 @@ gimple_boolify (tree expr) } } - if (TREE_CODE (type) =3D=3D BOOLEAN_TYPE) - return expr; - switch (TREE_CODE (expr)) { case TRUTH_AND_EXPR: @@ -2851,6 +2852,8 @@ gimple_boolify (tree expr) default: /* Other expressions that get here must have boolean values, but might need to be converted to the appropriate mode. */ + if (TREE_CODE (type) =3D=3D BOOLEAN_TYPE) + return expr; Why do you need to move this? return fold_convert_loc (loc, boolean_type_node, expr); } } @@ -2865,7 +2868,7 @@ gimplify_pure_cond_expr (tree *expr_p, g enum gimplify_status ret, tret; enum tree_code code; - cond =3D gimple_boolify (COND_EXPR_COND (expr)); + cond =3D COND_EXPR_COND (expr) =3D gimple_boolify (COND_EXPR_COND (expr)= ); why change COND_EXPR_COND? /* We need to handle && and || specially, as their gimplification creates pure cond_expr, thus leading to an infinite cycle otherwise. = */ @@ -2937,6 +2940,7 @@ gimplify_cond_expr (tree *expr_p, gimple enum tree_code pred_code; gimple_seq seq =3D NULL; + TREE_OPERAND (*expr_p, 0) =3D gimple_boolify (TREE_OPERAND (*expr_p, 0)); So you are doing it here - why do it everywhere else? I'd like to see this change at exactly _one_ place. /* If this COND_EXPR has a value, copy the values into a temporary within the arms. */ if (!VOID_TYPE_P (type)) @@ -4276,6 +4280,7 @@ gimplify_modify_expr_rhs (tree *expr_p, false); case COND_EXPR: + /* If we're assigning to a non-register type, push the assignment down into the branches. This is mandatory for ADDRESSABLE types, since we cannot generate temporaries for such, but it saves a @@ -4287,6 +4292,7 @@ gimplify_modify_expr_rhs (tree *expr_p, tree cond =3D *from_p; tree result =3D *to_p; + TREE_OPERAND (cond, 0) =3D gimple_boolify (TREE_OPERAND (cond, 0)); ret =3D gimplify_expr (&result, pre_p, post_p, is_gimple_lvalue, fb_lvalue); if (ret !=3D GS_ERROR) @@ -4710,6 +4716,7 @@ gimplify_boolean_expr (tree *expr_p, loc { /* Preserve the original type of the expression. */ tree type =3D TREE_TYPE (*expr_p); + *expr_p =3D gimple_boolify (*expr_p); *expr_p =3D build3 (COND_EXPR, type, *expr_p, fold_convert_loc (locus, type, boolean_true_node), @@ -6762,6 +6769,13 @@ gimplify_expr (tree *expr_p, gimple_seq case TRUTH_ANDIF_EXPR: case TRUTH_ORIF_EXPR: + if (TREE_CODE (TREE_TYPE (*expr_p)) !=3D BOOLEAN_TYPE) + { + tree type =3D TREE_TYPE (*expr_p); + *expr_p =3D fold_convert (type, gimple_boolify (*expr_p)); + ret =3D GS_OK; + break; + } Huh, that doesn't make sense. Why convert back to the original type? I know COND_EXPRs don't have a boolean value type-wise, but they do have one semantically. It looks like you are just papering over the inconsistencies at a lot of places instead of trying to fix them during gimplification. /* Pass the source location of the outer expression. */ ret =3D gimplify_boolean_expr (expr_p, saved_location); break; @@ -7203,6 +7217,30 @@ gimplify_expr (tree *expr_p, gimple_seq case TRUTH_AND_EXPR: case TRUTH_OR_EXPR: case TRUTH_XOR_EXPR: + if (TREE_CODE (TREE_TYPE (*expr_p)) !=3D BOOLEAN_TYPE) + { + tree type =3D TREE_TYPE (*expr_p); + *expr_p =3D fold_convert (type, gimple_boolify (*expr_p)); + ret =3D GS_OK; + break; + } + /* Call it to make sure that operands are boolified, too. */ + *expr_p =3D gimple_boolify (*expr_p); + switch (TREE_CODE (*expr_p)) + { + case TRUTH_AND_EXPR: + TREE_SET_CODE (*expr_p, BIT_AND_EXPR); + break; + case TRUTH_OR_EXPR: + TREE_SET_CODE (*expr_p, BIT_IOR_EXPR); + break; + case TRUTH_XOR_EXPR: + TREE_SET_CODE (*expr_p, BIT_XOR_EXPR); + break; + default: + break; + } + /* Classified as tcc_expression. */ goto expr_2; Please also change verify_gimple_assign_binary to barf on the TRUTH_* codes. We don't want other passes re-introducing them. Richard. > Regards, > Kai >