From patchwork Tue May 10 19:26:12 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kai Tietz X-Patchwork-Id: 95030 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]) by ozlabs.org (Postfix) with SMTP id 8E970B6EE9 for ; Wed, 11 May 2011 06:26:31 +1000 (EST) Received: (qmail 10364 invoked by alias); 10 May 2011 19:26:29 -0000 Received: (qmail 10355 invoked by uid 22791); 10 May 2011 19:26:28 -0000 X-SWARE-Spam-Status: No, hits=0.3 required=5.0 tests=AWL, BAYES_50, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, RFC_ABUSE_POST X-Spam-Check-By: sourceware.org Received: from mail-qw0-f47.google.com (HELO mail-qw0-f47.google.com) (209.85.216.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 10 May 2011 19:26:13 +0000 Received: by qwh5 with SMTP id 5so4450666qwh.20 for ; Tue, 10 May 2011 12:26:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.229.65.218 with SMTP id k26mr6515021qci.270.1305055572535; Tue, 10 May 2011 12:26:12 -0700 (PDT) Received: by 10.229.33.209 with HTTP; Tue, 10 May 2011 12:26:12 -0700 (PDT) In-Reply-To: References: <4DC95A03.9050704@gnu.org> Date: Tue, 10 May 2011 21:26:12 +0200 Message-ID: Subject: Re: [patch gimplifier]: Boolify more strict conditional expressions and transform simple form to binary From: Kai Tietz To: Richard Guenther Cc: Paolo Bonzini , GCC Patches X-IsSubscribed: yes 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 2011/5/10 Kai Tietz : > 2011/5/10 Richard Guenther : >> On Tue, May 10, 2011 at 5:30 PM, Paolo Bonzini wrote: >>> On 05/10/2011 05:23 PM, Richard Guenther wrote: >>>> >>>> I suppose you have testcases for all the cases you looked at, please >>>> add some that cover these corner cases. >>> >>> Also, there is quite some tree-vrp.c dead code with these changes. Removing >>> the TRUTH_*_CODE handling in VRP will help finding more places where the >>> middle-end is building boolean operations.  There should be testcases >>> covering these parts of VRP. >> >> Btw, you can split the patch into two pieces - first, make TRUTH_* >> expressions correctly typed (take boolean typed operands and procude >> a boolean typed result) and verify that in verify_gimple_assign_binary. >> A second patch than can do the s/TRUTH_/BIT_/ substitution during >> gimplification.  That way the first (and more difficult) part doesn't get >> too big with unrelated changes. >> >> Richard. >> >>> Paolo >>> >> > > Well, I think I found one big issue here about booified expression of > condition. The gimple_boolify needs to handle COND_EXPR in more > detail. As if a conditional expression has to be boolified, it means > its condition and its other operands need to be boolified, too. And > this is for sure one cause, why I see for ANDIF/ORIF and the truth > AND|OR|XOR none boolean types. > > I will continue on that. > > To split this seems to make sense, as I have to touch much more areas > for the TRUTH to BIT conversion. > > Regards, > Kai > So I use this thread for first part of the series of patches. This one improves boolean type-logic during gimplification. To gimple_boolify the handling for COND_EXPR are added, and in general it is tried to do boolean operation just on boolean type. As sadly fold-const (and here in special fold_truth_not_expr (), which doesn't provide by default boolean type and uses instead operand-type, which is IMHO a major flaw here. But well, there are some comments indicating that this behavior is required due some other quirks. But this is for sure something to be cleaned up) produces truth operations with wrong type, which are in calculations not necessarily identifyable as truth ops anymore, this patch makes sure that for truth AND|OR|XOR original type remains. 2011-05-10 Kai Tietz * gimplify.c (gimple_boolify): Handle COND_EXPR and make sure that even if type is BOOLEAN for TRUTH-opcodes the operands getting boolified. (gimplify_expr): Boolify operand condition for COND_EXPR. Boolify truth opcodes AND, ANDIF, OR, ORIF, and XOR. Additional take care that we keep expression's type. Ok for apply? Regards, Kai Index: gcc/gcc/gimplify.c =================================================================== --- gcc.orig/gcc/gimplify.c 2011-05-10 18:31:40.000000000 +0200 +++ gcc/gcc/gimplify.c 2011-05-10 21:14:49.106340400 +0200 @@ -2824,11 +2824,20 @@ gimple_boolify (tree expr) } } - if (TREE_CODE (type) == BOOLEAN_TYPE) - return expr; - switch (TREE_CODE (expr)) { + case COND_EXPR: + /* If we have a conditional expression, which shall be + boolean, take care we boolify also their left and right arm. */ + if (TREE_OPERAND (expr, 2) != NULL_TREE && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 2)))) + TREE_OPERAND (expr, 2) = gimple_boolify (TREE_OPERAND (expr, 2)); + if (TREE_OPERAND (expr, 1) != NULL_TREE && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 1)))) + TREE_OPERAND (expr, 1) = gimple_boolify (TREE_OPERAND (expr, 1)); + TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0)); + if (TREE_CODE (type) == BOOLEAN_TYPE) + return expr; + return fold_convert_loc (loc, boolean_type_node, expr); + case TRUTH_AND_EXPR: case TRUTH_OR_EXPR: case TRUTH_XOR_EXPR: @@ -2851,6 +2860,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) == BOOLEAN_TYPE) + return expr; return fold_convert_loc (loc, boolean_type_node, expr); } } @@ -6714,6 +6725,7 @@ gimplify_expr (tree *expr_p, gimple_seq break; case COND_EXPR: + TREE_OPERAND (*expr_p, 0) = gimple_boolify (TREE_OPERAND (*expr_p, 0)); ret = gimplify_cond_expr (expr_p, pre_p, fallback); /* C99 code may assign to an array in a structure value of a @@ -6762,6 +6774,17 @@ gimplify_expr (tree *expr_p, gimple_seq case TRUTH_ANDIF_EXPR: case TRUTH_ORIF_EXPR: + /* This shouldn't happen, but due fold-const (and here especially + fold_truth_not_expr) happily uses operand type and doesn't + automatically uses boolean_type as result, this happens. */ + if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE) + { + tree type = TREE_TYPE (*expr_p); + *expr_p = fold_convert (type, gimple_boolify (*expr_p)); + ret = GS_OK; + break; + } + *expr_p = gimple_boolify (*expr_p); /* Pass the source location of the outer expression. */ ret = gimplify_boolean_expr (expr_p, saved_location); break; @@ -7203,6 +7226,17 @@ gimplify_expr (tree *expr_p, gimple_seq case TRUTH_AND_EXPR: case TRUTH_OR_EXPR: case TRUTH_XOR_EXPR: + /* This shouldn't happen, but due fold-const (and here especially + fold_truth_not_expr) happily uses operand type and doesn't + automatically uses boolean_type as result, this happens. */ + if (TREE_CODE (TREE_TYPE (*expr_p)) != BOOLEAN_TYPE) + { + tree type = TREE_TYPE (*expr_p); + *expr_p = fold_convert (type, gimple_boolify (*expr_p)); + ret = GS_OK; + break; + } + *expr_p = gimple_boolify (*expr_p); /* Classified as tcc_expression. */ goto expr_2;