Re: Do not use TYPE_CANONICAL in useless_type_conversion

[Richard Biener <rguenther@suse.de>]

On Mon, 5 Oct 2015, Jan Hubicka wrote:

> > >+  /* For aggregates compare only the size and mode.  Accesses to fields do have
> > >+     a type information by themselves and thus we only care if we can i.e.
> > >+     use the types in move operations.  */
> > >    else if (AGGREGATE_TYPE_P (inner_type)
> > >  	   && TREE_CODE (inner_type) == TREE_CODE (outer_type))
> > >-    return false;
> > >+    return (!TYPE_SIZE (outer_type)
> > >+	    || (TYPE_SIZE (inner_type)
> > >+		&& operand_equal_p (TYPE_SIZE (inner_type),
> > >+				    TYPE_SIZE (outer_type), 0)));
> > >+
> > >+  else if (TREE_CODE (inner_type) == OFFSET_TYPE
> > >+	   && TREE_CODE (inner_type) == TREE_CODE (outer_type))
> > >+    return useless_type_conversion_p (TREE_TYPE (outer_type),
> > >+				      TREE_TYPE (inner_type))
> > >+	   && useless_type_conversion_p
> > >+	        (TYPE_OFFSET_BASETYPE (outer_type),
> > >+		 TYPE_OFFSET_BASETYPE (inner_type));
> > >
> > 
> > The comment says the mode is compared, but I don't see that in the
> > code. Which is right?

The comment is wrong.

> > Also, wouldn't the final condition be clearer written as
> > 
> > > +  else if (TREE_CODE (inner_type) == OFFSET_TYPE
> > > +	   && TREE_CODE (outer_type) == OFFSET_TYPE)
> 
> Updated in my local copy, thanks!

The patch works for me but I'm not sure about the store_expr_with_bounds
change.  Where does the actual copying take place?  adjust_address_nv
just adjusts the mode ...

Index: gcc/expr.c
===================================================================
--- gcc/expr.c  (revision 228514)
+++ gcc/expr.c  (working copy)
@@ -5462,7 +5462,7 @@ store_expr_with_bounds (tree exp, rtx ta
     {
       if (GET_MODE (temp) != GET_MODE (target) && GET_MODE (temp) != 
VOIDmode)
        {
-         if (GET_MODE (target) == BLKmode)
+         if (GET_MODE (target) == BLKmode || MEM_P (target))
            {
              /* Handle calls that return BLKmode values in registers.  */
              if (REG_P (temp) && TREE_CODE (exp) == CALL_EXPR)

works for me as well (for the testcase that ICEd for you) and it
definitely will emit the copy.  Basically if it is a MEM the mode
should be irrelevant (do we have BLKmode things that are _not_ MEMs?).

Anyway, your expr.c hunk looks wrong (no copy) and I'll let somebody
more familiar with that area do the sort-out.  Unfortunately
that ICEing testcase isn't a runtime one ;)

Richard.