This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
tree_ssa_useless_type_conversion is too optimistic
- From: Richard Guenther <rguenth at tat dot physik dot uni-tuebingen dot de>
- To: gcc at gcc dot gnu dot org
- Date: Sun, 15 May 2005 14:04:14 +0200
- Subject: tree_ssa_useless_type_conversion is too optimistic
In fact, tree_ssa_useless_type_conversion_1 contradicts itself if given
the two (C language) pointer types "char *" and "const char *". Asking
the langhook if they are compatible, it says no. But then in the
section that reads
/* 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)
&& TYPE_REF_CAN_ALIAS_ALL (inner_type)
== TYPE_REF_CAN_ALIAS_ALL (outer_type)
&& lang_hooks.types_compatible_p (TREE_TYPE (inner_type),
TREE_TYPE (outer_type)))
return true;
where, from the comment I read that we want to ignore casts of the form
T* -> T& or T& -> T*. Of course the logic applies to const T* -> T*,
too and we end up asking the C frontend if "char" and "const char" are
compatible which it happily says yes to (they share the main variant
"char"). So we return true for the compatibiltiy of the pointer types
based on the compatibiltiy of the pointed to types, which looks wrong.
Either we should constrain this test with something like
Index: tree-ssa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa.c,v
retrieving revision 2.94
diff -c -3 -p -r2.94 tree-ssa.c
*** tree-ssa.c 8 May 2005 15:07:22 -0000 2.94
--- tree-ssa.c 15 May 2005 11:43:35 -0000
*************** tree_ssa_useless_type_conversion_1 (tree
*** 885,890 ****
--- 885,891 ----
so strip conversions that just switch between them. */
else if (POINTER_TYPE_P (inner_type)
&& POINTER_TYPE_P (outer_type)
+ && TREE_CODE (inner_type) != TREE_CODE (outer_type)
&& TYPE_REF_CAN_ALIAS_ALL (inner_type)
== TYPE_REF_CAN_ALIAS_ALL (outer_type)
&& lang_hooks.types_compatible_p (TREE_TYPE (inner_type),
i.e. only use this form if we really deal with pointer vs. reference
thing, or we should actually test what we want to test -- compatibility
with ignoring the pointer/reference distinction with something like
Index: tree-ssa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa.c,v
retrieving revision 2.94
diff -c -3 -p -r2.94 tree-ssa.c
*** tree-ssa.c 8 May 2005 15:07:22 -0000 2.94
--- tree-ssa.c 15 May 2005 11:50:36 -0000
*************** tree_ssa_useless_type_conversion_1 (tree
*** 882,895 ****
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)
! && TYPE_REF_CAN_ALIAS_ALL (inner_type)
! == TYPE_REF_CAN_ALIAS_ALL (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
--- 882,901 ----
return true;
/* Pointers and references are equivalent once we get to GENERIC,
! so strip conversions that just switch between compatible
! pointer counterparts of them. */
else if (POINTER_TYPE_P (inner_type)
&& POINTER_TYPE_P (outer_type)
! && TREE_CODE (inner_type) != TREE_CODE (outer_type))
! {
! if (TREE_CODE (inner_type) == REFERENCE_TYPE)
! inner_type = build_pointer_type (TREE_TYPE (inner_type));
! else
! outer_type = build_pointer_type (TREE_TYPE (outer_type));
!
! if (lang_hooks.types_compatible_p (inner_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
Note that applying either of these variants, we loose the capability
to do the optimization g++.dg/opt/temp1.C is testing which strongly
hints at that we need to implement this optimization elsewhere instead
of doing possibly bogous transformations in
tree_ssa_useless_type_conversion and/or fold_indirect_ref_1 (note that
the testing was with type-santized fold_indirect_ref_1 for which I
attached the used patch as reference).
We also loose for g++.dg/tree-ssa/ssa-sra-1.C - somewhere we need to
add the ability to fold const-casts from rhs(!) only.
Any objections/preference for applying either of the above patches?
Thanks for any hints out of this type-hell,
Richard.
Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.580
diff -c -3 -p -r1.580 fold-const.c
*** fold-const.c 14 May 2005 15:42:01 -0000 1.580
--- fold-const.c 15 May 2005 12:01:54 -0000
*************** fold_indirect_ref_1 (tree t)
*** 11417,11432 ****
tree optype = TREE_TYPE (op);
/* *&p => p */
if (lang_hooks.types_compatible_p (type, optype))
! return op;
/* *(foo *)&fooarray => fooarray[0] */
else if (TREE_CODE (optype) == ARRAY_TYPE
&& lang_hooks.types_compatible_p (type, TREE_TYPE (optype)))
{
tree type_domain = TYPE_DOMAIN (optype);
tree min_val = size_zero_node;
if (type_domain && TYPE_MIN_VALUE (type_domain))
min_val = TYPE_MIN_VALUE (type_domain);
! return build4 (ARRAY_REF, type, op, min_val, NULL_TREE, NULL_TREE);
}
}
--- 11418,11444 ----
tree optype = TREE_TYPE (op);
/* *&p => p */
if (lang_hooks.types_compatible_p (type, optype))
! {
! if (TREE_TYPE (op) == type)
! return op;
! else
! return fold_build1 (VIEW_CONVERT_EXPR, type, op);
! }
/* *(foo *)&fooarray => fooarray[0] */
else if (TREE_CODE (optype) == ARRAY_TYPE
&& lang_hooks.types_compatible_p (type, TREE_TYPE (optype)))
{
tree type_domain = TYPE_DOMAIN (optype);
tree min_val = size_zero_node;
+ tree tmp;
if (type_domain && TYPE_MIN_VALUE (type_domain))
min_val = TYPE_MIN_VALUE (type_domain);
! tmp = build4 (ARRAY_REF, TREE_TYPE (optype),
! op, min_val, NULL_TREE, NULL_TREE);
! if (TREE_TYPE (tmp) == type)
! return tmp;
! else
! return fold_build1 (VIEW_CONVERT_EXPR, type, tmp);
}
}
*************** fold_indirect_ref_1 (tree t)
*** 11436,11446 ****
{
tree type_domain;
tree min_val = size_zero_node;
sub = build_fold_indirect_ref (sub);
type_domain = TYPE_DOMAIN (TREE_TYPE (sub));
if (type_domain && TYPE_MIN_VALUE (type_domain))
min_val = TYPE_MIN_VALUE (type_domain);
! return build4 (ARRAY_REF, type, sub, min_val, NULL_TREE, NULL_TREE);
}
return NULL_TREE;
--- 11448,11464 ----
{
tree type_domain;
tree min_val = size_zero_node;
+ tree tmp;
sub = build_fold_indirect_ref (sub);
type_domain = TYPE_DOMAIN (TREE_TYPE (sub));
if (type_domain && TYPE_MIN_VALUE (type_domain))
min_val = TYPE_MIN_VALUE (type_domain);
! tmp = build4 (ARRAY_REF, TREE_TYPE (TREE_TYPE (subtype)),
! sub, min_val, NULL_TREE, NULL_TREE);
! if (TREE_TYPE (tmp) == type)
! return tmp;
! else
! return fold_build1 (VIEW_CONVERT_EXPR, type, tmp);
}
return NULL_TREE;