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]

[tree-ssa] Fix problems with void * and GIMPLE typecasts [patch]


This patch started as a fix for a problem in the handling of void *.  We
were creating memory tags for those pointers, and since they can never
be dereferenced, their memory tags were never renamed, causing copy-prop
to introduce them unrenamed.

In the fix, I added a check in copy-prop that would make sure we are not
copy propagating pointers with different memory tags.  Two pointers have
different memory tags if the alias analyzer has determined that they
can't alias each other.

Essentially, GIMPLE is emitting either assignments or conditional
expressions between pointers of non-conflicting alias types, without
casting.  Since the two pointers have different non-alias-conflicting
types, they each get a different memory tag.

So, if the typecast operations aren't there, we will happily
copy-propagate those pointers.  If the final pointer is then
de-referenced, we will be looking at the wrong memory tag (probably
un-renamed), causing ICEs, or (worse) bad code generation.

For now, I have added a test in may_propagate_copy().  However, I would
rather have GIMPLE emit the proper type casts to avoid confusion.  There
is a FIXME note with an explanation of the problem and where I found it.

Both Java and C were causing this.  Jeff S., Jason, any thoughts on how
to best handle this?

Bootstrapped and tested ia64, x86 and amd64.


Diego.

	* tree-dfa.c (find_vars_r): Do not consider 'void *' pointers as
	dereferenced when scanning function call arguments.
	* tree-flow-inline.h (may_propagate_copy): Block propagation of
	pointers when they have different memory tags.
	* tree-ssa-copyprop.c (propagate_copy): When copy propagating
	pointers, abort if the two pointers don't have identical memory
	tags.

testsuite/ChangeLog.tree-ssa

	* gcc.dg/tree-ssa/20030917-2.c: New test.

Index: tree-dfa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/tree-dfa.c,v
retrieving revision 1.1.4.159
diff -d -c -p -r1.1.4.159 tree-dfa.c
*** tree-dfa.c	14 Sep 2003 13:36:12 -0000	1.1.4.159
--- tree-dfa.c	17 Sep 2003 14:15:29 -0000
*************** find_vars_r (tree *tp, int *walk_subtree
*** 2454,2460 ****
        for (op = TREE_OPERAND (t, 1); op; op = TREE_CHAIN (op))
  	{
  	  tree arg = TREE_VALUE (op);
! 	  if (SSA_VAR_P (arg) && POINTER_TYPE_P (TREE_TYPE (arg)))
  	    {
  	      walk_state->is_indirect_ref = 1;
  	      add_referenced_var (arg, walk_state);
--- 2454,2462 ----
        for (op = TREE_OPERAND (t, 1); op; op = TREE_CHAIN (op))
  	{
  	  tree arg = TREE_VALUE (op);
! 	  if (SSA_VAR_P (arg)
! 	      && POINTER_TYPE_P (TREE_TYPE (arg))
! 	      && !VOID_TYPE_P (TREE_TYPE (TREE_TYPE (arg))))
  	    {
  	      walk_state->is_indirect_ref = 1;
  	      add_referenced_var (arg, walk_state);
Index: tree-flow-inline.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/tree-flow-inline.h,v
retrieving revision 1.1.2.49
diff -d -c -p -r1.1.2.49 tree-flow-inline.h
*** tree-flow-inline.h	13 Sep 2003 20:21:26 -0000	1.1.2.49
--- tree-flow-inline.h	17 Sep 2003 14:15:29 -0000
*************** is_unchanging_value (tree val)
*** 528,533 ****
--- 528,580 ----
  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))->mem_tag;
+       tree mt_orig = var_ann (SSA_NAME_VAR (orig))->mem_tag;
+       if (mt_dest && mt_orig && mt_dest != mt_orig)
+ 	return false;
+     }
+ 
    return (!SSA_NAME_OCCURS_IN_ABNORMAL_PHI (dest)
  	  && (TREE_CODE (orig) != SSA_NAME
  	      || !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (orig))
Index: tree-ssa-copyprop.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/tree-ssa-copyprop.c,v
retrieving revision 1.1.2.13
diff -d -c -p -r1.1.2.13 tree-ssa-copyprop.c
*** tree-ssa-copyprop.c	6 Sep 2003 13:39:24 -0000	1.1.2.13
--- tree-ssa-copyprop.c	17 Sep 2003 14:15:29 -0000
*************** propagate_copy (tree *op_p, tree var, tr
*** 223,230 ****
--- 223,235 ----
      {
        var_ann_t new_ann = var_ann (SSA_NAME_VAR (var));
        var_ann_t orig_ann = var_ann (SSA_NAME_VAR (*op_p));
+ 
        if (new_ann->mem_tag == NULL_TREE)
  	new_ann->mem_tag = orig_ann->mem_tag;
+       else if (orig_ann->mem_tag == NULL_TREE)
+ 	orig_ann->mem_tag = new_ann->mem_tag;
+       else if (new_ann->mem_tag != orig_ann->mem_tag)
+ 	abort ();
      }
  
    *op_p = var;
Index: testsuite/gcc.dg/tree-ssa/20030917-2.c
===================================================================
RCS file: 20030917-2.c
diff -N 20030917-2.c
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- testsuite/gcc.dg/tree-ssa/20030917-2.c	17 Sep 2003 17:40:44 -0000	1.1.2.1
***************
*** 0 ****
--- 1,40 ----
+ /* This test was causing an ICE in DCE because we were allowing void *
+    pointers to have a memory tag, which we were copying when doing copy
+    propagation.  Since void * can never be de-referenced, its memory tag
+    was never renamed.  */
+ 
+ /* { dg-do compile } */
+ /* { dg-options "-O -ftree-dominator-opts" } */
+ 
+ struct operands_d
+ {
+   int *def_op;
+ };
+ 
+ typedef unsigned int size_t;
+ 
+ void
+ gt_ggc_mx_operands_d (void *x_p)
+ {
+   struct operands_d *const x = (struct operands_d *) x_p;
+   if ((*x).def_op != ((void *) 0))
+     {
+       size_t i0;
+       do
+ 	{
+ 	  const void *const a__ = ((*x).def_op);
+ 	  if (a__ != ((void *) 0) && a__ != (void *) 1)
+ 	    ggc_set_mark (a__);
+ 	}
+       while (0);
+       for (i0 = 0; i0 < (size_t) (1); i0++)
+ 	{
+ 	  do
+ 	    {
+ 	      if ((void *) (*x).def_op[i0] != ((void *) 0))
+ 		gt_ggc_mx_lang_tree_node ((*x).def_op[i0]);
+ 	    }
+ 	  while (0);
+ 	}
+     }
+ }


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