Re: Do not use TYPE_CANONICAL in useless_type_conversion

[Jan Hubicka <hubicka at ucw dot cz>]

> > On Fri, 2 Oct 2015, Jan Hubicka wrote:
> > 
> > > > There must be a reason why I allowed modes to differ there btw ;)
> > > 
> > > Thinking about it, I guess reason is that incomplete types do not have
> > > resonable modes set, so requiring modes to match will prevent complete
> > > and incomplete types to match.
> > 
> > Hmm.  I still think that the mode shouldn't be relevant.  Isn't this
> > only about the cases where the aggregate ends up not having its address
> > taken and thus we don't allocate a stack slot for it but end up
> > using a register with mode?  ISTR expansion has special cases dealing
> > with some of the mismatch cases and a fallback spilling to the stack.
> > 
> > I think we should fix that rather than disallowing aggregate assignments
> > (which are memory ops to GIMPLE) based on the mode of the aggregate.
> 
> THe ICE we get is:
> +===========================GNAT BUG DETECTED==============================+^M
> | 6.0.0 20150929 (experimental) (powerpc64-unknown-linux-gnu) GCC error:   |^M
> | in convert_move, at expr.c:282                                           |^M
> | Error detected around /home/jh/trunk/gcc/testsuite/gnat.dg/overriding_ops.ads:7:4|^M
> | Please submit a bug report; see http://gcc.gnu.org/bugs.html.            |^M
> | Use a subject line meaningful to you and us to track the bug.            |^M
> | Include the entire contents of this bug box in the report.               |^M
> | Include the exact command that you entered.                              |^M
> | Also include sources listed below.                                       |^M
> +==========================================================================+^M
> 
> which is:
> void
> convert_move (rtx to, rtx from, int unsignedp)
> {
>   machine_mode to_mode = GET_MODE (to);
>   machine_mode from_mode = GET_MODE (from);
>   int to_real = SCALAR_FLOAT_MODE_P (to_mode);
>   int from_real = SCALAR_FLOAT_MODE_P (from_mode);
>   enum insn_code code;
>   rtx libcall;
> 
>   /* rtx code for making an equivalent value.  */
>   enum rtx_code equiv_code = (unsignedp < 0 ? UNKNOWN
>                               : (unsignedp ? ZERO_EXTEND : SIGN_EXTEND));
> 
> 
>   gcc_assert (to_real == from_real);
>   gcc_assert (to_mode != BLKmode);
>   gcc_assert (from_mode != BLKmode);
> 
> so specifically we do not allow any RTL conversion from and to BLKmode and if
> one of them is, we die. I guess it is because convert_move does not know size
> of argument and does not expect it to be equivalent.  We can fix this earlier
> upstream when expanding assignment by adding appropriate subreg I think, but I
> would rather do things step by step - first get with removal of TYPE_CANONICAL
> and doing this second. I think one needs to fix expand_move and also probably an
> expansion of calls/memory statemetns that also take loads/stores as parameters.

Hi,
this is variant of patch I am testing. (it passed testing on ppc64-linux for Ada
that only trips the code, so I guess it will pass).  The convert_move is called
from store_expr_with_bounds which already knows how to handle stores to BLKmode.
It seems natural to extend it by stores from BLKmode that makes it possible to
not care about TYPE_MODE of aggregate when defining gimple types.

OK if testing passes?

Honza

	* expr.c (store_expr_with_bounds): Handle aggregate moves from
	BLKmode.
	* gimple-expr.c (useless_type_conversion_p): Do not use TYPE_CANONICAL
	to define gimple type system; compare aggregates only by size.
Index: expr.c
===================================================================
--- expr.c	(revision 228267)
+++ expr.c	(working copy)
@@ -5462,7 +5462,12 @@ 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 (temp) == BLKmode && GET_MODE (target) != BLKmode)
+	    {
+	      gcc_assert (AGGREGATE_TYPE_P (TREE_TYPE (exp)));
+	      temp = simplify_gen_subreg (GET_MODE (target), temp, BLKmode, 0);
+	    }
+	  else if (GET_MODE (target) == BLKmode)
 	    {
 	      /* Handle calls that return BLKmode values in registers.  */
 	      if (REG_P (temp) && TREE_CODE (exp) == CALL_EXPR)
Index: gimple-expr.c
===================================================================
--- gimple-expr.c	(revision 228267)
+++ gimple-expr.c	(working copy)
@@ -87,11 +87,6 @@ useless_type_conversion_p (tree outer_ty
   if (inner_type == outer_type)
     return true;
 
-  /* If we know the canonical types, compare them.  */
-  if (TYPE_CANONICAL (inner_type)
-      && TYPE_CANONICAL (inner_type) == TYPE_CANONICAL (outer_type))
-    return true;
-
   /* Changes in machine mode are never useless conversions unless we
      deal with aggregate types in which case we defer to later checks.  */
   if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type)
@@ -270,12 +265,23 @@ useless_type_conversion_p (tree outer_ty
       return true;
     }
 
-  /* For aggregates we rely on TYPE_CANONICAL exclusively and require
-     explicit conversions for types involving to be structurally
-     compared types.  */
+  /* For aggregates compare only the size.  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));
 
   return false;
 }