This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch tree-optimization]: Normalize compares X ==/!= 1 -> X !=/== 0 for truth valued X


2011/7/13 Richard Guenther <richard.guenther@gmail.com>:
> On Wed, Jul 13, 2011 at 11:00 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> Hello,
>>
>> this patch fixes that for replaced uses, we call fold_stmt_inplace. Additionally
>> it adds to fold_gimple_assign the canonical form for X !=/== 1 -> X ==/!= 0 for
>> X with one-bit precision type.
>>
>> ChangeLog gcc/
>>
>> 2011-07-13 ?Kai Tietz ?<ktietz@redhat.com>
>>
>> ? ? ? ?* gimple-fold.c (fold_gimple_assign): Add normalization
>> ? ? ? ?for compares of 1-bit integer precision operands.
>> ? ? ? ?* tree-ssa-propagate.c (replace_uses_in): Call
>> ? ? ? ?fold_stmt_inplace on modified statement.
>
> err - sure not. ?The caller already does that.
>
> Breakpoint 5, substitute_and_fold (get_value_fn=0xc269ae <get_value>,
> ? ?fold_fn=0, do_dce=1 '\001')
> ? ?at /space/rguenther/src/svn/trunk/gcc/tree-ssa-propagate.c:1134
> 1134 ? ? ? ? ? ? ?if (get_value_fn)
> D.2696_8 = a_1(D) != D.2704_10;
>
> (gdb) n
> 1135 ? ? ? ? ? ? ? ?did_replace |= replace_uses_in (stmt, get_value_fn);
> (gdb)
> 1138 ? ? ? ? ? ? ?if (did_replace)
> (gdb) call debug_gimple_stmt (stmt)
> D.2696_8 = a_1(D) != 1;
>
> (gdb) p did_replace
> $1 = 1 '\001'
> (gdb) n
> 1139 ? ? ? ? ? ? ? ?fold_stmt (&oldi);
>
> so figure out why fold_stmt does not do its work instead. ?Which I
> quickly checked in gdb and it dispatches to fold_binary with
> boolean-typed arguments as a_1 != 1 where you can see the
> canonical form for this is !(int) a_1 because of a bug I think.
>
> ? ? ?/* bool_var != 1 becomes !bool_var. */
> ? ? ?if (TREE_CODE (TREE_TYPE (arg0)) == BOOLEAN_TYPE && integer_onep (arg1)
> ? ? ? ? ?&& code == NE_EXPR)
> ? ? ? ?return fold_build1_loc (loc, TRUTH_NOT_EXPR, type,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?fold_convert_loc (loc, type, arg0));
>
> at least I don't see why we need to convert arg0 to the type of the
> comparison.

Well, this type-cast is required by C specification - integer
autopromotion - AFAIR.  So I don't think FE maintainer would be happy
about this change.
Nevertheless I saw this pattern before, and was wondering why we check
here for boolean_type at all. This might be in Ada-case even a latent
bug due type-precision, and it prevents signed case detection too.
IMHO this check should look like that:

      /* bool_var != 1 becomes !bool_var. */
      if (INTEGRAL_TYPE_P (TREE_TYPE (arg0))
         && TYPE_PRECISION (TREE_TYPE (arg0)) == 1
         && integer_onep (arg1) && code == NE_EXPR)
        return fold_build1_loc (loc, TRUTH_NOT_EXPR, type,
                            fold_convert_loc (loc, type, arg0));

For thie BIT_NOT_EXPR variant, the cast of arg0 would be of course
false, as ~(bool) is of course different in result then ~(int)

> You need to improve your debugging skills and see why existing
> transformations are not working before adding new ones.

I work on that.

Kai


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]