On Mon, Oct 10, 2011 at 5:07 PM, Kai Tietz <ktiet...@googlemail.com> wrote: > 2011/10/10 Richard Guenther <richard.guent...@gmail.com>: >> On Mon, Oct 10, 2011 at 4:06 PM, Kai Tietz <ktiet...@googlemail.com> wrote: >>> 2011/10/10 Richard Guenther <richard.guent...@gmail.com>: >>>> On Mon, Oct 10, 2011 at 2:29 PM, Kai Tietz <ktiet...@googlemail.com> wrote: >>>>> Recent patch had a thinko on rhs of inner lhs check for TRUTH-IF. It >>>>> has to be checked that the LHS code is same as outer CODE, as >>>>> otherwise we wouldn't apply different TRUTH-IF only on inner RHS of >>>>> LHS, which is of course wrong. >>>>> >>>>> Index: gcc/gcc/fold-const.c >>>>> =================================================================== >>>>> --- gcc.orig/gcc/fold-const.c >>>>> +++ gcc/gcc/fold-const.c >>>>> @@ -111,14 +111,13 @@ static tree decode_field_reference (loca >>>>> tree *, tree *); >>>>> static int all_ones_mask_p (const_tree, int); >>>>> static tree sign_bit_p (tree, const_tree); >>>>> -static int simple_operand_p (const_tree); >>>>> +static int simple_operand_p (tree); >>>>> static tree range_binop (enum tree_code, tree, tree, int, tree, int); >>>>> static tree range_predecessor (tree); >>>>> static tree range_successor (tree); >>>>> static tree fold_range_test (location_t, enum tree_code, tree, tree, >>>>> tree); >>>>> static tree fold_cond_expr_with_comparison (location_t, tree, tree, >>>>> tree, tree); >>>>> static tree unextend (tree, int, int, tree); >>>>> -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree); >>>>> static tree optimize_minmax_comparison (location_t, enum tree_code, >>>>> tree, tree, tree); >>>>> static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *); >>>>> @@ -3500,7 +3499,7 @@ optimize_bit_field_compare (location_t l >>>>> return lhs; >>>>> } >>>>> >>>>> -/* Subroutine for fold_truthop: decode a field reference. >>>>> +/* Subroutine for fold_truth_andor_1: decode a field reference. >>>>> >>>>> If EXP is a comparison reference, we return the innermost reference. >>>>> >>>>> @@ -3668,17 +3667,43 @@ sign_bit_p (tree exp, const_tree val) >>>>> return NULL_TREE; >>>>> } >>>>> >>>>> -/* Subroutine for fold_truthop: determine if an operand is simple enough >>>>> +/* Subroutine for fold_truth_andor_1: determine if an operand is simple >>>>> enough >>>>> to be evaluated unconditionally. */ >>>>> >>>>> static int >>>>> -simple_operand_p (const_tree exp) >>>>> +simple_operand_p (tree exp) >>>>> { >>>>> + enum tree_code code; >>>>> /* Strip any conversions that don't change the machine mode. */ >>>>> STRIP_NOPS (exp); >>>>> >>>>> + code = TREE_CODE (exp); >>>>> + >>>>> + /* Handle some trivials */ >>>>> + if (TREE_CODE_CLASS (code) == tcc_comparison) >>>>> + return (tree_could_trap_p (exp) >>>>> + && simple_operand_p (TREE_OPERAND (exp, 0)) >>>>> + && simple_operand_p (TREE_OPERAND (exp, 1))); >>>> >>>> And that's still wrong. >>>> >>>> Stopped reading here. >>>> >>>> Richard. >>> >>> Oh, there is a not missing. I didn't spot that, sorry. >>> >>> To the point why we need to handle comparisons within simple_operand_p. >>> >>> If we reject comparisons and logical not here, we won't have any >>> branching optimization anymore, as this the patch moves into >>> fold_truthandor. >>> >>> The result with rejecting in simple_operand_p compares and logic-not >>> provides for the following example: >> >> But you change what simple_operand_p accepts and thus change what >> fold_truthop accepts as operands to its simplifications. >> >> Richard. > > Well, not really. I assume you mean fold_truth_andor_1 (aka fold_truthop). > > It checks for > ... > if (TREE_CODE_CLASS (lcode) != tcc_comparison > || TREE_CODE_CLASS (rcode) != tcc_comparison) > return 0; > ... > before checking for simple_operand_p. So there is actual no change. > It might be of some interest here to add in a different patch support > for logic-not, but well, this is would be material for a different > patch. > So, it won't operate on anything else then comparisons as before.
Sure, because simple_operand_p is checked on the comparison operands, not the comparison itself. Richard. > Kai >