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
>

Reply via email to