This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
fix mis-matched type propagation from dom
- From: Richard Henderson <rth at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Wed, 7 Jul 2004 13:29:27 -0700
- Subject: 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. */