[RFC] Tightening up the type system

[RFC] Tightening up the type system

[Diego Novillo]

[-- Attachment #1: Type: text/plain, Size: 1820 bytes --]


Over the last few days I have found various places in the optimizers
where binary expressions and assignments had operands of non-compatible
types.

For instance, out of the gimplifier we were getting an assignment like
'iftmp.256 = 0B', where iftmp.256 was 'some_type *' and 0B was
'void *'.

We would then fail to propagate iftmp.256 because its constant value is
of the wrong type (may_propagate_copy would reject it).  This was caused
by tree_ssa_useless_type_conversion_1 stripping a (void *) from the
original FE trees.

In other cases we were generating boolean 'true'/'false' constants when
we really needed to be generating integer values (because of ABI
requirements).

So, as an experiment I added the attached patch to the statement
verifier.  Needless to say, we ICE almost immediately.  I first had to
exclude all the shift operators (tree.def states that the types may be
different).  But now I am getting a failure building libgcov.o on this
assignment

result = D.4720 + &__gcov_var.buffer[0];

(gdb) ptu t.exp.operands[0].common.type
const gcov_unsigned_t *
(gdb) ptu t.exp.operands[1].common.type
gcov_unsigned_t *

lang_hook.compatible_types_p is rejecting those two types because of the
const qualifier.

My question to the FE folks is: what are the right semantics for
checking MODIFY_EXPR?  Is compatible_types_p too strict?  Should we have
had a cast operation in the above assignment?

Not having the right types is increasingly getting in the way of the
optimizers because we use compatible_types_p quite often to validate
propagation opportunities.

In fact, I would like to get to the point where we can simply call
'gcc_assert (lang_hook.compatible_types_p (dest, orig))' when doing
propagation.  Ideally, all the necessary type conversions should be
exposed in the IL.


Thanks.  Diego.



[-- Attachment #2: 00 --]
[-- Type: text/x-patch, Size: 1712 bytes --]

Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.59
diff -d -u -p -r2.59 tree-cfg.c
--- tree-cfg.c	28 Sep 2004 07:59:50 -0000	2.59
+++ tree-cfg.c	28 Sep 2004 15:27:58 -0000
@@ -3062,6 +3062,7 @@ static tree
 verify_expr (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
 {
   tree t = *tp, x;
+  enum tree_code code;
 
   if (TYPE_P (t))
     *walk_subtrees = 0;
@@ -3074,7 +3075,8 @@ verify_expr (tree *tp, int *walk_subtree
          && !is_gimple_val (TREE_OPERAND (t, N)))		\
        { error (MSG); return TREE_OPERAND (t, N); }} while (0)
 
-  switch (TREE_CODE (t))
+  code = TREE_CODE (t);
+  switch (code)
     {
     case SSA_NAME:
       if (SSA_NAME_IN_FREE_LIST (t))
@@ -3092,6 +3094,13 @@ verify_expr (tree *tp, int *walk_subtree
 	  error ("GIMPLE register modified with BIT_FIELD_REF");
 	  return t;
 	}
+
+      if (!lang_hooks.types_compatible_p (TREE_TYPE (TREE_OPERAND (t, 0)),
+					  TREE_TYPE (TREE_OPERAND (t, 1))))
+	{
+	  error ("Incompatible types in an assignment");
+	  return t;
+	}
       break;
 
     case ADDR_EXPR:
@@ -3218,6 +3227,17 @@ verify_expr (tree *tp, int *walk_subtree
     case BIT_AND_EXPR:
       CHECK_OP (0, "Invalid operand to binary operator");
       CHECK_OP (1, "Invalid operand to binary operator");
+
+      if (code != LSHIFT_EXPR
+	  && code != RSHIFT_EXPR
+	  && code != LROTATE_EXPR
+	  && code != RROTATE_EXPR
+          && !lang_hooks.types_compatible_p (TREE_TYPE (TREE_OPERAND (t, 0)),
+					     TREE_TYPE (TREE_OPERAND (t, 1))))
+	{
+	  error ("Incompatible types in a binary operator");
+	  return t;
+	}
       break;
 
     default:

[RFC] Tightening up the type system

[Andrew Haley]

Diego Novillo writes:
 > 
 > 
 > My question to the FE folks is: what are the right semantics for
 > checking MODIFY_EXPR?  Is compatible_types_p too strict?  Should we have
 > had a cast operation in the above assignment?
 > 
 > Not having the right types is increasingly getting in the way of the
 > optimizers because we use compatible_types_p quite often to validate
 > propagation opportunities.

It's an interesting view.  I'm pretty sure that we violate this is the
Java FE in a few places, but perhaps we shouldn't.  The trouble is
that the GENERIC type system has never been so well-defined.

I'm sure that enforcing this would break things and it would take some
time to find and fix them all. 

Andrew.

Re: [RFC] Tightening up the type system

[Diego Novillo]

On Tue, 2004-09-28 at 13:05, Andrew Haley wrote:
> Diego Novillo writes:
>  > 
>  > 
>  > My question to the FE folks is: what are the right semantics for
>  > checking MODIFY_EXPR?  Is compatible_types_p too strict?  Should we have
>  > had a cast operation in the above assignment?
>  > 
>  > Not having the right types is increasingly getting in the way of the
>  > optimizers because we use compatible_types_p quite often to validate
>  > propagation opportunities.
> 
> It's an interesting view.  I'm pretty sure that we violate this is the
> Java FE in a few places, but perhaps we shouldn't.  The trouble is
> that the GENERIC type system has never been so well-defined.
> 
We use lang_hooks for adhering to the FE type system.  We don't really
have a GENERIC/GIMPLE type system.

Maybe we should if we lowered GIMPLE one more step, but for now we tie
the optimizers to the FE's type system.

> I'm sure that enforcing this would break things and it would take some
> time to find and fix them all. 
> 
As a stop-gap measure, we could probably ask the gimplifier to
fold_convert operands into the right type.


Diego.

Re: [RFC] Tightening up the type system

[Andrew Haley]

Diego Novillo writes:
 > On Tue, 2004-09-28 at 13:05, Andrew Haley wrote:
 > > Diego Novillo writes:
 > >  > 
 > >  > 
 > >  > My question to the FE folks is: what are the right semantics for
 > >  > checking MODIFY_EXPR?  Is compatible_types_p too strict?  Should we have
 > >  > had a cast operation in the above assignment?
 > >  > 
 > >  > Not having the right types is increasingly getting in the way of the
 > >  > optimizers because we use compatible_types_p quite often to validate
 > >  > propagation opportunities.
 > > 
 > > It's an interesting view.  I'm pretty sure that we violate this is the
 > > Java FE in a few places, but perhaps we shouldn't.  The trouble is
 > > that the GENERIC type system has never been so well-defined.
 > > 
 > We use lang_hooks for adhering to the FE type system.  We don't really
 > have a GENERIC/GIMPLE type system.
 > 
 > Maybe we should if we lowered GIMPLE one more step, but for now we tie
 > the optimizers to the FE's type system.

Yeah, but surely the front end can generate trees that are not legal
in its own type system.  For example, Java doesn't have a (void*)
that's assignable to anything, but we generate trees of type (void*)
when lowering.

Andrew.

Re: [RFC] Tightening up the type system

[Jan Hubicka]

> Diego Novillo writes:
>  > 
>  > 
>  > My question to the FE folks is: what are the right semantics for
>  > checking MODIFY_EXPR?  Is compatible_types_p too strict?  Should we have
>  > had a cast operation in the above assignment?
>  > 
>  > Not having the right types is increasingly getting in the way of the
>  > optimizers because we use compatible_types_p quite often to validate
>  > propagation opportunities.
> 
> It's an interesting view.  I'm pretty sure that we violate this is the
> Java FE in a few places, but perhaps we shouldn't.  The trouble is
> that the GENERIC type system has never been so well-defined.
> 
> I'm sure that enforcing this would break things and it would take some
> time to find and fix them all. 

Also note that in the archives should be simply type checking code for
verify_stmts I implemented while back.  It also had trouble witht he way
we elliminate useless NOP_EXPR conversions

Honza

Re: [RFC] Tightening up the type system

[Nathan Sidwell]

Andrew Haley wrote:
> Diego Novillo writes:
>  > 
>  > 
>  > My question to the FE folks is: what are the right semantics for
>  > checking MODIFY_EXPR?  Is compatible_types_p too strict?  Should we have
>  > had a cast operation in the above assignment?
>  > 
>  > Not having the right types is increasingly getting in the way of the
>  > optimizers because we use compatible_types_p quite often to validate
>  > propagation opportunities.
> 
> It's an interesting view.  I'm pretty sure that we violate this is the
> Java FE in a few places, but perhaps we shouldn't.  The trouble is
> that the GENERIC type system has never been so well-defined.
> 
> I'm sure that enforcing this would break things and it would take some
> time to find and fix them all. 

It'd force us to treat an lvalue packed field as incompatible to an
unpacked one, at last.

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

Re: [RFC] Tightening up the type system

[Tom Tromey]

>>>>> "Andrew" == Andrew Haley <aph@redhat.com> writes:

>> > It's an interesting view.  I'm pretty sure that we violate this is the
>> > Java FE in a few places, but perhaps we shouldn't.  The trouble is
>> > that the GENERIC type system has never been so well-defined.

I'd like to see us continue toward the goal of having generic and
gimple be well defined and fairly strict about checking -- ideally
with documentation preceding implementation.  I think we can make it
much easier to write front ends, and much easier to look at some
random tree and decide whether or not it is actually valid.

So, I'm in favor of adding the checks, with the caveat that if it
turns into a big morass it should perhaps be put off until the next
stage 1.

>> Maybe we should if we lowered GIMPLE one more step, but for now we tie
>> the optimizers to the FE's type system.

Andrew> Yeah, but surely the front end can generate trees that are not legal
Andrew> in its own type system.  For example, Java doesn't have a (void*)
Andrew> that's assignable to anything, but we generate trees of type (void*)
Andrew> when lowering.

I'd say that, in an ideal situation, it is up to the front end to
decide what kind of type system it exposes to gimple.  For Java this
might be "java types plus void*", the semantics of which we must
define.  In particular it seems to me that this need not be identical
to the language's type system.

Tom

Re: [RFC] Tightening up the type system

[Diego Novillo]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

On Tue, 2004-09-28 at 15:57, Tom Tromey wrote:

> So, I'm in favor of adding the checks, with the caveat that if it
> turns into a big morass it should perhaps be put off until the next
> stage 1.
> 
It's certainly looking that way.  Attached is the WIP patch.  The main
changes are in the gimplifier (as we break down expressions, we need to
adjust the types of the operands) and in some optimizers that were not
emitting operands with the right types (e.g., ivopts).

I then ran into limitations in builtin processing.  We have several
regressions in testsuite/builtins mostly because we are no longer able
to fold memmove to memcpy because we convert

memmove (&a, &b, n)

to

a.1 = (void *) &a;
b.1 = (const void *) &b;
memmove (a.1, b.1, n);

which we then fail to fold into memcpy because we don't recognize that
a.1 and b.1 really are non-overlapping.  Perhaps this optimization
should be moved up into the tree optimizers.  I'm not quite sure where
exactly this happens.

I will keep working on this from time to time, mostly to keep the patch
alive until 4.1 opens.  If anyone is interested in picking it up, let me
know.


Diego.

[-- Attachment #2: 20040930-type-checking.diff --]
[-- Type: text/x-patch, Size: 19403 bytes --]

Index: Makefile.in
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Makefile.in,v
retrieving revision 1.1404
diff -d -c -p -u -r1.1404 Makefile.in
--- Makefile.in	29 Sep 2004 09:47:41 -0000	1.1404
+++ Makefile.in	30 Sep 2004 16:28:30 -0000
@@ -1719,11 +1719,11 @@ tree-ssa-loop-ivopts.o : tree-ssa-loop-i
    $(SYSTEM_H) $(RTL_H) $(TREE_H) $(TM_P_H) $(CFGLOOP_H) varray.h $(EXPR_H) \
    output.h diagnostic.h $(TIMEVAR_H) $(TM_H) coretypes.h $(TREE_DUMP_H) \
    tree-pass.h $(GGC_H) $(RECOG_H) insn-config.h $(HASHTAB_H) $(SCEV_H) \
-   cfgloop.h $(PARAMS_H)
+   cfgloop.h $(PARAMS_H) langhooks.h
 tree-ssa-loop-manip.o : tree-ssa-loop-manip.c $(TREE_FLOW_H) $(CONFIG_H) \
    $(SYSTEM_H) $(RTL_H) $(TREE_H) $(TM_P_H) $(CFGLOOP_H) \
    output.h diagnostic.h $(TIMEVAR_H) $(TM_H) coretypes.h $(TREE_DUMP_H) \
-   tree-pass.h cfglayout.h $(SCEV_H)
+   tree-pass.h cfglayout.h $(SCEV_H) langhooks.h
 tree-ssa-loop-im.o : tree-ssa-loop-im.c $(TREE_FLOW_H) $(CONFIG_H) \
    $(SYSTEM_H) $(RTL_H) $(TREE_H) $(TM_P_H) $(CFGLOOP_H) domwalk.h $(PARAMS_H)\
    output.h diagnostic.h $(TIMEVAR_H) $(TM_H) coretypes.h $(TREE_DUMP_H) \
Index: gimplify.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.82
diff -d -c -p -u -r2.82 gimplify.c
--- gimplify.c	30 Sep 2004 01:22:05 -0000	2.82
+++ gimplify.c	30 Sep 2004 16:28:31 -0000
@@ -177,6 +177,99 @@ gimple_current_bind_expr (void)
   return gimplify_ctxp->current_bind_expr;
 }
 
+/* Adjust the types of operands to binary expression EXPR so that they
+   are compatible with EXPR's type or with each other. This is
+   necessary because as we start breaking appart complex expressions
+   with type casts, the types of the underlying operands may be wrong.
+
+   Consider, for instance the expression
+   
+   (const unsigned char *) (unsigned char *) fde + 8B
+
+   Initially, the FE generates the constant '8' with type 'unsigned
+   char *', but when we take the LHS of '+' and replace it with a
+   temporary of type 'const unsigned char *', we need to change the
+   type of '8' accordingly.
+
+   PRE_P and POST_P are as in gimplify_expr.  */
+
+static void
+adjust_operand_types (tree expr, tree *pre_p, tree *post_p)
+{
+  tree op0, op1, op0_type, op1_type, expr_type, cast_val;
+  enum tree_code code = TREE_CODE (expr);
+
+  gcc_assert (TREE_CODE_LENGTH (code) == 2);
+
+  expr_type = TREE_TYPE (expr);
+
+  op0 = TREE_OPERAND (expr, 0);
+  op0_type = TREE_TYPE (op0);
+
+  op1 = TREE_OPERAND (expr, 1);
+  op1_type = TREE_TYPE (op1);
+
+  if (code == MODIFY_EXPR)
+    {
+      /* If the type of the RHS is not compatible with that of the
+	 LHS, change its type.  */
+      if (!lang_hooks.types_compatible_p (op0_type, op1_type))
+	{
+	  cast_val = fold_convert (op0_type, op1);
+	  op1 = get_initialized_tmp_var (cast_val, pre_p, post_p);
+	  TREE_OPERAND (expr, 1) = op1;
+	}
+    }
+  else if (TREE_CODE_CLASS (code) == tcc_binary)
+    {
+      /* Ignore expressions whose operands may be of different types
+	 and expressions that return a type different from their
+	 operands.  */
+      if (code == LSHIFT_EXPR
+	  || code == RSHIFT_EXPR
+	  || code == LROTATE_EXPR
+	  || code == RROTATE_EXPR
+	  || code == COMPLEX_EXPR)
+	return;
+
+      /* Otherwise, make sure that both operands have a type
+	 compatible with EXPR's type.  */
+      if (!lang_hooks.types_compatible_p (expr_type, op0_type))
+	{
+	  cast_val = fold_convert (expr_type, op0);
+	  op0 = get_initialized_tmp_var (cast_val, pre_p, post_p);
+	  TREE_OPERAND (expr, 0) = op0;
+	}
+
+      if (!lang_hooks.types_compatible_p (expr_type, op1_type))
+	{
+	  cast_val = fold_convert (expr_type, op1);
+	  op1 = get_initialized_tmp_var (cast_val, pre_p, post_p);
+	  TREE_OPERAND (expr, 1) = op1;
+	}
+    }
+  else if (TREE_CODE_CLASS (code) == tcc_comparison
+	   || code == TRUTH_ANDIF_EXPR
+	   || code == TRUTH_ORIF_EXPR
+	   || code == TRUTH_AND_EXPR
+	   || code == TRUTH_OR_EXPR
+	   || code == TRUTH_XOR_EXPR)
+    {
+      /* Make sure the two operands have compatible types.  By
+	 convention, only modify the type for the RHS of the
+	 comparison, if needed.  */
+      if (!lang_hooks.types_compatible_p (op0_type, op1_type))
+	{
+	  cast_val = fold_convert (op0_type, op1);
+	  op1 = get_initialized_tmp_var (cast_val, pre_p, post_p);
+	  TREE_OPERAND (expr, 1) = op1;
+	}
+    }
+  else
+    gcc_unreachable ();
+}
+
+
 /* Returns true iff there is a COND_EXPR between us and the innermost
    CLEANUP_POINT_EXPR.  This info is used by gimple_push_cleanup.  */
 
@@ -2826,6 +2919,10 @@ gimplify_modify_expr (tree *expr_p, tree
   if (ret == GS_ERROR)
     return ret;
 
+  /* Once the LHS and RHS have been gimplified, make sure that their
+     types are compatible.  */
+  adjust_operand_types (*expr_p, pre_p, post_p);
+
   /* Now see if the above changed *from_p to something we handle specially.  */
   ret = gimplify_modify_expr_rhs (expr_p, from_p, to_p, pre_p, post_p,
 				  want_value);
@@ -3902,6 +3999,9 @@ gimplify_expr (tree *expr_p, tree *pre_p
 		r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
 				    post_p, is_gimple_val, fb_rvalue);
 
+		if (r0 != GS_ERROR && r1 != GS_ERROR)
+		  adjust_operand_types (*expr_p, pre_p, post_p);
+
 		ret = MIN (r0, r1);
 		break;
 	      }
Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.59
diff -d -c -p -u -r2.59 tree-cfg.c
--- tree-cfg.c	28 Sep 2004 07:59:50 -0000	2.59
+++ tree-cfg.c	30 Sep 2004 16:28:31 -0000
@@ -3062,6 +3062,7 @@ static tree
 verify_expr (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
 {
   tree t = *tp, x;
+  enum tree_code code;
 
   if (TYPE_P (t))
     *walk_subtrees = 0;
@@ -3074,7 +3075,8 @@ verify_expr (tree *tp, int *walk_subtree
          && !is_gimple_val (TREE_OPERAND (t, N)))		\
        { error (MSG); return TREE_OPERAND (t, N); }} while (0)
 
-  switch (TREE_CODE (t))
+  code = TREE_CODE (t);
+  switch (code)
     {
     case SSA_NAME:
       if (SSA_NAME_IN_FREE_LIST (t))
@@ -3092,6 +3094,13 @@ verify_expr (tree *tp, int *walk_subtree
 	  error ("GIMPLE register modified with BIT_FIELD_REF");
 	  return t;
 	}
+
+      if (!lang_hooks.types_compatible_p (TREE_TYPE (TREE_OPERAND (t, 0)),
+					  TREE_TYPE (TREE_OPERAND (t, 1))))
+	{
+	  error ("Incompatible types in an assignment");
+	  return t;
+	}
       break;
 
     case ADDR_EXPR:
@@ -3216,8 +3225,21 @@ verify_expr (tree *tp, int *walk_subtree
     case BIT_IOR_EXPR:
     case BIT_XOR_EXPR:
     case BIT_AND_EXPR:
+    case COMPLEX_EXPR:
       CHECK_OP (0, "Invalid operand to binary operator");
       CHECK_OP (1, "Invalid operand to binary operator");
+
+      if (code != LSHIFT_EXPR
+	  && code != RSHIFT_EXPR
+	  && code != LROTATE_EXPR
+	  && code != RROTATE_EXPR
+	  && code != COMPLEX_EXPR
+          && !lang_hooks.types_compatible_p (TREE_TYPE (TREE_OPERAND (t, 0)),
+					     TREE_TYPE (TREE_OPERAND (t, 1))))
+	{
+	  error ("Incompatible types in a binary operator");
+	  return t;
+	}
       break;
 
     default:
Index: tree-flow.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-flow.h,v
retrieving revision 2.50
diff -d -c -p -u -r2.50 tree-flow.h
--- tree-flow.h	29 Sep 2004 21:23:35 -0000	2.50
+++ tree-flow.h	30 Sep 2004 16:28:32 -0000
@@ -579,7 +579,6 @@ extern void debug_tree_ssa_stats (void);
 extern void ssa_remove_edge (edge);
 extern edge ssa_redirect_edge (edge, basic_block);
 extern bool tree_ssa_useless_type_conversion (tree);
-extern bool tree_ssa_useless_type_conversion_1 (tree, tree);
 extern void verify_ssa (void);
 extern void delete_tree_ssa (void);
 extern void register_new_def (tree, varray_type *);
Index: tree-ssa-ccp.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-ccp.c,v
retrieving revision 2.47
diff -d -c -p -u -r2.47 tree-ssa-ccp.c
--- tree-ssa-ccp.c	29 Sep 2004 09:04:17 -0000	2.47
+++ tree-ssa-ccp.c	30 Sep 2004 16:28:32 -0000
@@ -460,6 +460,9 @@ replace_uses_in (tree stmt, bool *replac
       if (val->lattice_val != CONSTANT)
 	continue;
 
+      gcc_assert (lang_hooks.types_compatible_p (TREE_TYPE (tuse),
+						 TREE_TYPE (val->const_val)));
+
       if (TREE_CODE (stmt) == ASM_EXPR
 	  && !may_propagate_copy_into_asm (tuse))
 	continue;
@@ -486,6 +489,7 @@ replace_vuse_in (tree stmt, bool *replac
   bool replaced = false;
   vuse_optype vuses;
   use_operand_p vuse;
+  tree var;
   value *val;
 
   if (replaced_addresses_p)
@@ -499,13 +503,17 @@ replace_vuse_in (tree stmt, bool *replac
     return false;
 
   vuse = VUSE_OP_PTR (vuses, 0);
+  var = USE_FROM_PTR (vuse);
   val = get_value (USE_FROM_PTR (vuse));
 
   if (val->lattice_val == CONSTANT
       && TREE_CODE (stmt) == MODIFY_EXPR
       && DECL_P (TREE_OPERAND (stmt, 1))
-      && TREE_OPERAND (stmt, 1) == SSA_NAME_VAR (USE_FROM_PTR (vuse)))
+      && TREE_OPERAND (stmt, 1) == SSA_NAME_VAR (var))
     {
+      gcc_assert (lang_hooks.types_compatible_p (TREE_TYPE (var),
+						 TREE_TYPE (val->const_val)));
+
       TREE_OPERAND (stmt, 1) = val->const_val;
       replaced = true;
       if (POINTER_TYPE_P (TREE_TYPE (USE_FROM_PTR (vuse))) 
@@ -550,6 +558,10 @@ substitute_and_fold (void)
 	      if (! SSA_VAR_P (orig))
 		break;
 
+	      /* The call to may_propagate_copy is needed to prevent
+		 propagating constants coming from V_MUST_DEFs.  We
+		 will still need the original store to the PHI
+		 argument to properly take this PHI node out of SSA.  */
 	      new_val = get_value (orig);
 	      if (new_val->lattice_val == CONSTANT
 		  && may_propagate_copy (orig, new_val->const_val))
Index: tree-ssa-copy.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-copy.c,v
retrieving revision 2.18
diff -d -c -p -u -r2.18 tree-ssa-copy.c
--- tree-ssa-copy.c	30 Sep 2004 01:22:05 -0000	2.18
+++ tree-ssa-copy.c	30 Sep 2004 16:28:33 -0000
@@ -64,7 +64,7 @@ may_propagate_copy (tree dest, tree orig
   tree type_o = TREE_TYPE (orig);
 
   /* Do not copy between types for which we *do* need a conversion.  */
-  if (!tree_ssa_useless_type_conversion_1 (type_d, type_o))
+  if (!lang_hooks.types_compatible_p (type_d, type_o))
     return false;
 
   /* FIXME.  GIMPLE is allowing pointer assignments and comparisons of
Index: tree-ssa-dom.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-dom.c,v
retrieving revision 2.57
diff -d -c -p -u -r2.57 tree-ssa-dom.c
--- tree-ssa-dom.c	29 Sep 2004 23:08:32 -0000	2.57
+++ tree-ssa-dom.c	30 Sep 2004 16:28:33 -0000
@@ -2343,6 +2343,9 @@ eliminate_redundant_computations (struct
       && (TREE_CODE (cached_lhs) != SSA_NAME
 	  || may_propagate_copy (*expr_p, cached_lhs)))
     {
+      if (TREE_CODE (cached_lhs) != SSA_NAME)
+	gcc_assert (may_propagate_copy (*expr_p, cached_lhs));
+
       if (dump_file && (dump_flags & TDF_DETAILS))
 	{
 	  fprintf (dump_file, "  Replaced redundant expr '");
@@ -2522,8 +2525,6 @@ cprop_operand (tree stmt, use_operand_p 
   val = SSA_NAME_VALUE (op);
   if (val && TREE_CODE (val) != VALUE_HANDLE)
     {
-      tree op_type, val_type;
-
       /* Do not change the base variable in the virtual operand
 	 tables.  That would make it impossible to reconstruct
 	 the renamed virtual operand if we later modify this
@@ -2539,38 +2540,17 @@ cprop_operand (tree stmt, use_operand_p 
 	  && !may_propagate_copy_into_asm (op))
 	return false;
 
-      /* Get the toplevel type of each operand.  */
-      op_type = TREE_TYPE (op);
-      val_type = TREE_TYPE (val);
-
-      /* While both types are pointers, get the type of the object
-	 pointed to.  */
-      while (POINTER_TYPE_P (op_type) && POINTER_TYPE_P (val_type))
-	{
-	  op_type = TREE_TYPE (op_type);
-	  val_type = TREE_TYPE (val_type);
-	}
-
-      /* Make sure underlying types match before propagating a constant by
-	 converting the constant to the proper type.  Note that convert may
-	 return a non-gimple expression, in which case we ignore this
-	 propagation opportunity.  */
-      if (TREE_CODE (val) != SSA_NAME)
-	{
-	  if (!lang_hooks.types_compatible_p (op_type, val_type))
-	    {
-	      val = fold_convert (TREE_TYPE (op), val);
-	      if (!is_gimple_min_invariant (val))
-		return false;
-	    }
-	}
-
       /* Certain operands are not allowed to be copy propagated due
 	 to their interaction with exception handling and some GCC
 	 extensions.  */
       else if (!may_propagate_copy (op, val))
 	return false;
 
+      /* Make sure underlying types match before propagating
+	 the new value.  */
+      gcc_assert (lang_hooks.types_compatible_p (TREE_TYPE (op),
+						 TREE_TYPE (val)));
+
       /* Dump details.  */
       if (dump_file && (dump_flags & TDF_DETAILS))
 	{
@@ -2879,10 +2859,14 @@ lookup_avail_expr (tree stmt, bool inser
 	  tree t = element->rhs;
 	  free (element);
 
+	  /* Note that the predicate may have a type other than
+	     'bool'.  This happens when this predicate is the return
+	     expression of a function returning 'bool' with an ABI that
+	     widens the return value.  */
 	  if (TREE_CODE (t) == EQ_EXPR)
-	    return boolean_false_node;
+	    return fold_convert (TREE_TYPE (t), boolean_false_node);
 	  else
-	    return boolean_true_node;
+	    return fold_convert (TREE_TYPE (t), boolean_true_node);
 	}
     }
 
Index: tree-ssa-loop-ivopts.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-loop-ivopts.c,v
retrieving revision 2.15
diff -d -c -p -u -r2.15 tree-ssa-loop-ivopts.c
--- tree-ssa-loop-ivopts.c	28 Sep 2004 07:59:52 -0000	2.15
+++ tree-ssa-loop-ivopts.c	30 Sep 2004 16:28:33 -0000
@@ -88,6 +88,7 @@ Software Foundation, 59 Temple Place - S
 #include "tree-scalar-evolution.h"
 #include "cfgloop.h"
 #include "params.h"
+#include "langhooks.h"
 
 /* The infinite cost.  */
 #define INFTY 10000000
@@ -2977,6 +2978,7 @@ may_eliminate_iv (struct loop *loop,
   edge exit;
   struct tree_niter_desc *niter, new_niter;
   tree wider_type, type, base;
+  tree var;
 
   /* For now just very primitive -- we work just for the single exit condition,
      and are quite conservative about the possible overflows.  TODO -- both of
@@ -3023,6 +3025,15 @@ may_eliminate_iv (struct loop *loop,
 			fold_convert (wider_type, new_niter.niter), 0))
     return false;
 
+  /* Adjust *BOUND to the type of the candidate variable.  */
+  var = var_at_stmt (loop, cand, use->stmt);
+  if (!lang_hooks.types_compatible_p (TREE_TYPE (var), TREE_TYPE (*bound)))
+    {
+      *bound = fold_convert (TREE_TYPE (var), *bound);
+      if (!is_gimple_val (*bound))
+	return false;
+    }
+
   return true;
 }
 
@@ -3960,7 +3971,14 @@ rewrite_address_base (block_stmt_iterato
      the variable to a new temporary.  */
   copy = build2 (MODIFY_EXPR, void_type_node, NULL_TREE, with);
   if (name)
-    new_name = duplicate_ssa_name (name, copy);
+    {
+      new_name = duplicate_ssa_name (name, copy);
+
+      /* Make sure that WITH has a compatible type with NEW_NAME.  */
+      if (!lang_hooks.types_compatible_p (TREE_TYPE (new_name),
+					  TREE_TYPE (with)))
+	TREE_OPERAND (copy, 1) = fold_convert (TREE_TYPE (new_name), with);
+    }
   else
     {
       new_var = create_tmp_var (TREE_TYPE (with), "ruatmp");
Index: tree-ssa-loop-manip.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-loop-manip.c,v
retrieving revision 2.9
diff -d -c -p -u -r2.9 tree-ssa-loop-manip.c
--- tree-ssa-loop-manip.c	28 Sep 2004 07:59:52 -0000	2.9
+++ tree-ssa-loop-manip.c	30 Sep 2004 16:28:33 -0000
@@ -36,6 +36,7 @@ Software Foundation, 59 Temple Place - S
 #include "tree-pass.h"
 #include "cfglayout.h"
 #include "tree-scalar-evolution.h"
+#include "langhooks.h"
 
 /* Creates an induction variable with value BASE + STEP * iteration in LOOP.
    It is expected that neither BASE nor STEP are shared with other expressions
@@ -91,6 +92,13 @@ create_iv (tree base, tree step, tree va
 	}
     }
 
+  /* Make sure that STEP is of a type compatible with VB.  */
+  if (!lang_hooks.types_compatible_p (TREE_TYPE (vb), TREE_TYPE (step)))
+    {
+      step = fold_convert (TREE_TYPE (vb), step);
+      gcc_assert (is_gimple_val (step));
+    }
+
   stmt = build2 (MODIFY_EXPR, void_type_node, va,
 		 build2 (incr_op, TREE_TYPE (base),
 			 vb, step));
Index: tree-ssa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa.c,v
retrieving revision 2.44
diff -d -c -p -u -r2.44 tree-ssa.c
--- tree-ssa.c	30 Sep 2004 01:22:06 -0000	2.44
+++ tree-ssa.c	30 Sep 2004 16:28:34 -0000
@@ -733,7 +733,7 @@ delete_tree_ssa (void)
 /* Return true if EXPR is a useless type conversion, otherwise return
    false.  */
 
-bool
+static bool
 tree_ssa_useless_type_conversion_1 (tree outer_type, tree inner_type)
 {
   /* If the inner and outer types are effectively the same, then
@@ -743,51 +743,6 @@ tree_ssa_useless_type_conversion_1 (tree
      || (lang_hooks.types_compatible_p (inner_type, outer_type)))
     return true;
 
-  /* If both types are pointers and the outer type is a (void *), then
-     the conversion is not necessary.  The opposite is not true since
-     that conversion would result in a loss of information if the
-     equivalence was used.  Consider an indirect function call where
-     we need to know the exact type of the function to correctly
-     implement the ABI.  */
-  else if (POINTER_TYPE_P (inner_type)
-           && POINTER_TYPE_P (outer_type)
-	   && TREE_CODE (TREE_TYPE (outer_type)) == VOID_TYPE)
-    return true;
-
-  /* Pointers and references are equivalent once we get to GENERIC,
-     so strip conversions that just switch between them.  */
-  else if (POINTER_TYPE_P (inner_type)
-           && POINTER_TYPE_P (outer_type)
-           && lang_hooks.types_compatible_p (TREE_TYPE (inner_type),
-					     TREE_TYPE (outer_type)))
-    return true;
-
-  /* If both the inner and outer types are integral types, then the
-     conversion is not necessary if they have the same mode and
-     signedness and precision, and both or neither are boolean.  Some
-     code assumes an invariant that boolean types stay boolean and do
-     not become 1-bit bit-field types.  Note that types with precision
-     not using all bits of the mode (such as bit-field types in C)
-     mean that testing of precision is necessary.  */
-  else if (INTEGRAL_TYPE_P (inner_type)
-           && INTEGRAL_TYPE_P (outer_type)
-	   && TYPE_MODE (inner_type) == TYPE_MODE (outer_type)
-	   && TYPE_UNSIGNED (inner_type) == TYPE_UNSIGNED (outer_type)
-	   && TYPE_PRECISION (inner_type) == TYPE_PRECISION (outer_type))
-    {
-      bool first_boolean = (TREE_CODE (inner_type) == BOOLEAN_TYPE);
-      bool second_boolean = (TREE_CODE (outer_type) == BOOLEAN_TYPE);
-      if (first_boolean == second_boolean)
-	return true;
-    }
-
-  /* Recurse for complex types.  */
-  else if (TREE_CODE (inner_type) == COMPLEX_TYPE
-	   && TREE_CODE (outer_type) == COMPLEX_TYPE
-	   && tree_ssa_useless_type_conversion_1 (TREE_TYPE (outer_type),
-						  TREE_TYPE (inner_type)))
-    return true;
-
   return false;
 }
 

Re: [RFC] Tightening up the type system

[Geoffrey Keating]

I actually have a bug report along these lines; compiling the following:

int
foo(double x, long double y)
{
 return  __builtin_isgreater (x, y);
}

crashes, because we end up creating an UNLE tree node with parameters
of two different types, and the RTL generator asserts.

Re: [RFC] Tightening up the type system

[Geoffrey Keating]

Diego Novillo <dnovillo@redhat.com> writes:

> In fact, I would like to get to the point where we can simply call
> 'gcc_assert (lang_hook.compatible_types_p (dest, orig))' when doing
> propagation.  Ideally, all the necessary type conversions should be
> exposed in the IL.

Should the validity of GIMPLE really depend on a language hook like
this?  Surely it would make more sense to have a
gimple_compatible_types_p that reflects what code that parses GIMPLE
can actually handle (presumably a superset of what each language can
generate).

Re: [RFC] Tightening up the type system

[Diego Novillo]

On Thu, 2004-09-30 at 15:58, Geoffrey Keating wrote:
> Diego Novillo <dnovillo@redhat.com> writes:
> 
> > In fact, I would like to get to the point where we can simply call
> > 'gcc_assert (lang_hook.compatible_types_p (dest, orig))' when doing
> > propagation.  Ideally, all the necessary type conversions should be
> > exposed in the IL.
> 
> Should the validity of GIMPLE really depend on a language hook like
> this?  Surely it would make more sense to have a
> gimple_compatible_types_p that reflects what code that parses GIMPLE
> can actually handle (presumably a superset of what each language can
> generate).
>
Perhaps.  Today I was thinking something along the lines of using
alias_sets_conflict_p() as our type compatibility test, instead.  That
would loosen the restrictions and allow mixing things like const type *
and type *.

Right now, we have a hybrid model where we try to remove "useless" type
conversions, but then validate propagation opportunities using
compatible_types_p. 


Diego.

Re: [RFC] Tightening up the type system

[Richard Kenner]

    It's an interesting view.  I'm pretty sure that we violate this is the
    Java FE in a few places, but perhaps we shouldn't.  The trouble is
    that the GENERIC type system has never been so well-defined.

That's true, however, I've always treated GENERIC's type system as
being very strict: the types must *exactly* agree.  Certainly the Ada
front end goes to a lot of trouble to make that happen in the trees it
generates.  I think there has been some code that assumes
type-correctness, but not much.

In GIMPLE, we relax this to allow any types that are type_compatible_p.

    I'm sure that enforcing this would break things and it would take some
    time to find and fix them all. 

On the other hand, *not* enforcing this looks like it's causing problems
for the optimizers.  Those problems are going to be harder to find and
fix than typing violations in the front end.

Re: [RFC] Tightening up the type system

[Richard Kenner]

    My question to the FE folks is: what are the right semantics for
    checking MODIFY_EXPR?  Is compatible_types_p too strict?  Should we
    have had a cast operation in the above assignment?

My view is the compatible_types_p is exactly the right condition.  We
certainly ought to be able to get there for scalars, though aggregate
types sometimes have some corner cases that might be tricky.

    In fact, I would like to get to the point where we can simply call
    'gcc_assert (lang_hook.compatible_types_p (dest, orig))' when doing
    propagation.  Ideally, all the necessary type conversions should be
    exposed in the IL.

I agree (strongly).  I've been pushing for this sort of consistency for
a long time.