Re: Add VIEW_CONVERT_EXPR to operand_equal_p

[Jan Hubicka <hubicka@ucw.cz>]

> >         * fold-const.c (operand_equal_p): Handle VIEW_CONVERT_EXPR.
> > Index: fold-const.c
> > ===================================================================
> > --- fold-const.c        (revision 228735)
> > +++ fold-const.c        (working copy)
> > @@ -2962,6 +2968,12 @@ operand_equal_p (const_tree arg0, const_
> >         case IMAGPART_EXPR:
> >           return OP_SAME (0);
> >
> > +       case VIEW_CONVERT_EXPR:
> > +         if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF))
> > +             && !types_compatible_p (TREE_TYPE (arg0), TREE_TYPE (arg1)))
> 
> Note that mixing the GIMPLE types_compatible_p into operand_equal_p which
> is supposed to handle GENERIC as well looks dangerous.  You short-cutted
> the type checks for OEP_ADDRESS_OF earlier so why do you need to
> preserve them here?
> 
> The test looks bogus anyway, but maybe it's just too early in the morning ;)
> Shouldn't it be && types_compatible_p (...) (instead of && !types_com...)?
> Otherwise it looks really weird.

I think it is as intended: If we do !OEP_ADDRESS_OF we want to know that types
are compatible. 

I am not quite sure this is needed - I wanted to mention that in an email, but
it was too late in night for me :)

In NOP_EXPR we check:

      /* Two conversions are equal only if signedness and modes match.  */
      switch (TREE_CODE (arg0))
        {
        CASE_CONVERT:
        case FIX_TRUNC_EXPR:
          if (TYPE_UNSIGNED (TREE_TYPE (arg0))
              != TYPE_UNSIGNED (TREE_TYPE (arg1)))
            return 0;
          break;
        default:
          break;
        }

And earlier we do:

      /* If both types don't have the same signedness, then we can't consider
         them equal.  We must check this before the STRIP_NOPS calls
         because they may change the signedness of the arguments.  As pointers
         strictly don't have a signedness, require either two pointers or
         two non-pointers as well.  */
      if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
          || POINTER_TYPE_P (TREE_TYPE (arg0))
                             != POINTER_TYPE_P (TREE_TYPE (arg1)))
        return 0;

So except for NOP_EXPR being stripped early this is bit redundant.

I wondered what happens if I match two VCEs of very different type. Say
vector of FLOATs and vector of INTEGERs and then convert them to same type that
would result in differnt value. But this is not terribly special for VCE
and probably would affect other expressions, too.

I noticed now that this probably this can't happen because we also do:

  /* This is needed for conversions and for COMPONENT_REF.
     Might as well play it safe and always test this.  */
  if (TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
      || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
      || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))
    return 0;

Which sounds somewhat conservative. At least should be skipped for OEP_ADDRESS_OF.
You earlier mentioned BLKmode vectors. What would happen in this case?

If I remember correctly. the check was not needed to pass testing, I am respawning testing
without the type check.

Honza
> 
> Richard.
> 
> > +           return false;
> > +         return OP_SAME (0);
> > +
> >         case TARGET_MEM_REF:
> >         case MEM_REF:
> >           if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)))