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]

fix mis-matched type propagation from dom


In a moderately patched tree, during stage1 build of real.c, I ran
into dom1 propagating a value into a PHI and discarding the char->int
cast that had been part of the assignment.  This was caught by the
rtl expander, via a move with mismatched modes.

I don't know how to replicate the problem without the other patches,
though via inspection it certainly seems the possibility is there.

I'm not certain that this is the best solution.  I would have thought
that dom wouldn't even attempt to copy-prop a rhs that included a cast.
With the desire to keep the "large pending cleanup" stack from getting
any deeper, I went with the big hammer solution.


r~


        * tree-flow-inline.h (may_propagate_copy): Move...
        * tree-ssa-copy.c (may_propagate_copy): ... here.  Fail if we
        attempt to copy between types requiring conversion.
        * tree-flow.h (may_propagate_copy): Update decl.
        * tree-ssa-dom.c (cprop_operand): Tidy redundant tests.

Index: gcc/tree-flow-inline.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-flow-inline.h,v
retrieving revision 2.13
diff -c -p -d -r2.13 tree-flow-inline.h
*** gcc/tree-flow-inline.h	30 Jun 2004 18:21:53 -0000	2.13
--- gcc/tree-flow-inline.h	7 Jul 2004 20:10:44 -0000
*************** is_label_stmt (tree t)
*** 467,563 ****
    return false;
  }
  
- /* Return true if we may propagate ORIG into DEST, false otherwise.  */
- static inline bool
- may_propagate_copy (tree dest, tree orig)
- {
-   /* FIXME.  GIMPLE is allowing pointer assignments and comparisons of
-      pointers that have different alias sets.  This means that these
-      pointers will have different memory tags associated to them.
-      
-      If we allow copy propagation in these cases, statements de-referencing
-      the new pointer will now have a reference to a different memory tag
-      with potentially incorrect SSA information.
- 
-      This was showing up in libjava/java/util/zip/ZipFile.java with code
-      like:
- 
-      	struct java.io.BufferedInputStream *T.660;
- 	struct java.io.BufferedInputStream *T.647;
- 	struct java.io.InputStream *is;
- 	struct java.io.InputStream *is.662;
- 	[ ... ]
- 	T.660 = T.647;
- 	is = T.660;	<-- This ought to be type-casted
- 	is.662 = is;
- 
-      Also, f/name.c exposed a similar problem with a COND_EXPR predicate
-      that was causing DOM to generate and equivalence with two pointers of
-      alias-incompatible types:
- 
-      	struct _ffename_space *n;
- 	struct _ffename *ns;
- 	[ ... ]
- 	if (n == ns)
- 	  goto lab;
- 	...
- 	lab:
- 	return n;
- 
-      I think that GIMPLE should emit the appropriate type-casts.  For the
-      time being, blocking copy-propagation in these cases is the safe thing
-      to do.  */
-   if (TREE_CODE (dest) == SSA_NAME
-       && TREE_CODE (orig) == SSA_NAME
-       && POINTER_TYPE_P (TREE_TYPE (dest))
-       && POINTER_TYPE_P (TREE_TYPE (orig)))
-     {
-       tree mt_dest = var_ann (SSA_NAME_VAR (dest))->type_mem_tag;
-       tree mt_orig = var_ann (SSA_NAME_VAR (orig))->type_mem_tag;
-       if (mt_dest && mt_orig && mt_dest != mt_orig)
- 	return false;
-     }
- 
-   /* If the destination is a SSA_NAME for a virtual operand, then we have
-      some special cases to handle.  */
-   if (TREE_CODE (dest) == SSA_NAME && !is_gimple_reg (dest))
-     {
-       /* If both operands are SSA_NAMEs referring to virtual operands, then
- 	 we can always propagate.  */
-       if (TREE_CODE (orig) == SSA_NAME)
- 	{
- 	  if (!is_gimple_reg (orig))
- 	    return true;
- 
- #ifdef ENABLE_CHECKING
- 	  /* If we have one real and one virtual operand, then something has
- 	     gone terribly wrong.  */
- 	  if (is_gimple_reg (orig))
- 	    abort ();
- #endif
- 	}
- 
-       /* We have a "copy" from something like a constant into a virtual
- 	 operand.  Reject these.  */
-       return false;
-     }
- 
-   /* If ORIG flows in from an abnormal edge, it cannot be propagated.  */
-   if (TREE_CODE (orig) == SSA_NAME
-       && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (orig))
-     return false;
- 
-   /* If DEST is an SSA_NAME that flows from an abnormal edge or if it
-      represents a hard register, then it cannot be replaced.  */
-   if (TREE_CODE (dest) == SSA_NAME
-       && (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (dest)
- 	  || DECL_HARD_REGISTER (SSA_NAME_VAR (dest))))
-     return false;
- 
-   /* Anything else is OK.  */
-   return true;
- }
- 
  /* Set the default definition for VAR to DEF.  */
  static inline void
  set_default_def (tree var, tree def)
--- 467,472 ----
Index: gcc/tree-flow.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-flow.h,v
retrieving revision 2.18
diff -c -p -d -r2.18 tree-flow.h
*** gcc/tree-flow.h	2 Jul 2004 07:34:30 -0000	2.18
--- gcc/tree-flow.h	7 Jul 2004 20:10:44 -0000
*************** extern void debug_dominator_optimization
*** 591,600 ****
  extern void propagate_value (use_operand_p, tree);
  extern void propagate_tree_value (tree *, tree);
  extern void replace_exp (use_operand_p, tree);
  
  /* In tree-flow-inline.h  */
  static inline int phi_arg_from_edge (tree, edge);
- static inline bool may_propagate_copy (tree, tree);
  static inline bool is_call_clobbered (tree);
  static inline void mark_call_clobbered (tree);
  
--- 591,600 ----
  extern void propagate_value (use_operand_p, tree);
  extern void propagate_tree_value (tree *, tree);
  extern void replace_exp (use_operand_p, tree);
+ extern bool may_propagate_copy (tree, tree);
  
  /* In tree-flow-inline.h  */
  static inline int phi_arg_from_edge (tree, edge);
  static inline bool is_call_clobbered (tree);
  static inline void mark_call_clobbered (tree);
  
Index: gcc/tree-ssa-copy.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-copy.c,v
retrieving revision 2.9
diff -c -p -d -r2.9 tree-ssa-copy.c
*** gcc/tree-ssa-copy.c	2 Jul 2004 00:51:00 -0000	2.9
--- gcc/tree-ssa-copy.c	7 Jul 2004 20:10:44 -0000
*************** Boston, MA 02111-1307, USA.  */
*** 55,60 ****
--- 55,157 ----
     APIs defined in this file.  */
  
  
+ /* Return true if we may propagate ORIG into DEST, false otherwise.  */
+ 
+ bool
+ may_propagate_copy (tree dest, tree orig)
+ {
+   tree type_d = TREE_TYPE (dest);
+   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))
+     return false;
+ 
+   /* FIXME.  GIMPLE is allowing pointer assignments and comparisons of
+      pointers that have different alias sets.  This means that these
+      pointers will have different memory tags associated to them.
+      
+      If we allow copy propagation in these cases, statements de-referencing
+      the new pointer will now have a reference to a different memory tag
+      with potentially incorrect SSA information.
+ 
+      This was showing up in libjava/java/util/zip/ZipFile.java with code
+      like:
+ 
+      	struct java.io.BufferedInputStream *T.660;
+ 	struct java.io.BufferedInputStream *T.647;
+ 	struct java.io.InputStream *is;
+ 	struct java.io.InputStream *is.662;
+ 	[ ... ]
+ 	T.660 = T.647;
+ 	is = T.660;	<-- This ought to be type-casted
+ 	is.662 = is;
+ 
+      Also, f/name.c exposed a similar problem with a COND_EXPR predicate
+      that was causing DOM to generate and equivalence with two pointers of
+      alias-incompatible types:
+ 
+      	struct _ffename_space *n;
+ 	struct _ffename *ns;
+ 	[ ... ]
+ 	if (n == ns)
+ 	  goto lab;
+ 	...
+ 	lab:
+ 	return n;
+ 
+      I think that GIMPLE should emit the appropriate type-casts.  For the
+      time being, blocking copy-propagation in these cases is the safe thing
+      to do.  */
+   if (TREE_CODE (dest) == SSA_NAME && TREE_CODE (orig) == SSA_NAME
+       && POINTER_TYPE_P (type_d) && POINTER_TYPE_P (type_o))
+     {
+       tree mt_dest = var_ann (SSA_NAME_VAR (dest))->type_mem_tag;
+       tree mt_orig = var_ann (SSA_NAME_VAR (orig))->type_mem_tag;
+       if (mt_dest && mt_orig && mt_dest != mt_orig)
+ 	return false;
+     }
+ 
+   /* If the destination is a SSA_NAME for a virtual operand, then we have
+      some special cases to handle.  */
+   if (TREE_CODE (dest) == SSA_NAME && !is_gimple_reg (dest))
+     {
+       /* If both operands are SSA_NAMEs referring to virtual operands, then
+ 	 we can always propagate.  */
+       if (TREE_CODE (orig) == SSA_NAME)
+ 	{
+ 	  if (!is_gimple_reg (orig))
+ 	    return true;
+ 
+ #ifdef ENABLE_CHECKING
+ 	  /* If we have one real and one virtual operand, then something has
+ 	     gone terribly wrong.  */
+ 	  if (is_gimple_reg (orig))
+ 	    abort ();
+ #endif
+ 	}
+ 
+       /* We have a "copy" from something like a constant into a virtual
+ 	 operand.  Reject these.  */
+       return false;
+     }
+ 
+   /* If ORIG flows in from an abnormal edge, it cannot be propagated.  */
+   if (TREE_CODE (orig) == SSA_NAME
+       && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (orig))
+     return false;
+ 
+   /* If DEST is an SSA_NAME that flows from an abnormal edge or if it
+      represents a hard register, then it cannot be replaced.  */
+   if (TREE_CODE (dest) == SSA_NAME
+       && (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (dest)
+ 	  || DECL_HARD_REGISTER (SSA_NAME_VAR (dest))))
+     return false;
+ 
+   /* Anything else is OK.  */
+   return true;
+ }
+ 
  /* Given two SSA_NAMEs, replace the annotations for the one referred to by OP 
     with VAR's annotations.
  
Index: gcc/tree-ssa-dom.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-dom.c,v
retrieving revision 2.22
diff -c -p -d -r2.22 tree-ssa-dom.c
*** gcc/tree-ssa-dom.c	3 Jul 2004 18:57:17 -0000	2.22
--- gcc/tree-ssa-dom.c	7 Jul 2004 20:10:44 -0000
*************** cprop_operand (stmt_ann_t ann, use_opera
*** 2906,2929 ****
  	  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 (!lang_hooks.types_compatible_p (op_type, val_type)
!            && TREE_CODE (val) != SSA_NAME)
  	{
! 	  val = fold_convert (TREE_TYPE (op), val);
! 	  if (!is_gimple_min_invariant (val)
! 	      && TREE_CODE (val) != SSA_NAME)
! 	    return false;
  	}
  
        /* Certain operands are not allowed to be copy propagated due
  	 to their interaction with exception handling and some GCC
  	 extensions.  */
!       if (TREE_CODE (val) == SSA_NAME
! 	  && !may_propagate_copy (op, val))
  	return false;
  
        /* Dump details.  */
--- 2906,2929 ----
  	  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;
  
        /* Dump details.  */


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