diff mbox

useless_type_conversion_p vs pointer sizes

Message ID 201201122309.q0CN9T8o027553@greed.delorie.com
State New
Headers show

Commit Message

DJ Delorie Jan. 12, 2012, 11:09 p.m. UTC
Another case where one address space may support multiple pointer
sizes, so conversions between such must be preserved.

	* tree-ssa.c (useless_type_conversion_p): Conversions between
	different-sized pointers aren't useless.

Comments

Richard Biener Jan. 13, 2012, 9:30 a.m. UTC | #1
On Fri, Jan 13, 2012 at 12:09 AM, DJ Delorie <dj@redhat.com> wrote:
>
> Another case where one address space may support multiple pointer
> sizes, so conversions between such must be preserved.
>
>        * tree-ssa.c (useless_type_conversion_p): Conversions between
>        different-sized pointers aren't useless.
>
> Index: tree-ssa.c
> ===================================================================
> --- tree-ssa.c  (revision 183139)
> +++ tree-ssa.c  (working copy)
> @@ -1192,12 +1192,17 @@ bool
>  useless_type_conversion_p (tree outer_type, tree inner_type)
>  {
>   /* Do the following before stripping toplevel qualifiers.  */
>   if (POINTER_TYPE_P (inner_type)
>       && POINTER_TYPE_P (outer_type))
>     {
> +      /* Do not lose casts between pointers of different precision.  */
> +      if (TYPE_PRECISION (outer_type)
> +         != TYPE_PRECISION (inner_type))
> +       return false;
> +
>       /* Do not lose casts between pointers to different address spaces.  */
>       if (TYPE_ADDR_SPACE (TREE_TYPE (outer_type))
>          != TYPE_ADDR_SPACE (TREE_TYPE (inner_type)))
>        return false;
>
>       /* If the outer type is (void *), the conversion is not necessary.  */

That should not be necessary as there is a mode check below.  Do you
hit the issue only when the VOID_TYPE_P check is true?  In that case
simply delete it - it has become obsolete.

Richard.
Tristan Gingold Jan. 13, 2012, 10:29 a.m. UTC | #2
On Jan 13, 2012, at 10:30 AM, Richard Guenther wrote:

> On Fri, Jan 13, 2012 at 12:09 AM, DJ Delorie <dj@redhat.com> wrote:
>> 
>> Another case where one address space may support multiple pointer
>> sizes, so conversions between such must be preserved.
>> 
>>        * tree-ssa.c (useless_type_conversion_p): Conversions between
>>        different-sized pointers aren't useless.
>> 
>> Index: tree-ssa.c
>> ===================================================================
>> --- tree-ssa.c  (revision 183139)
>> +++ tree-ssa.c  (working copy)
>> @@ -1192,12 +1192,17 @@ bool
>>  useless_type_conversion_p (tree outer_type, tree inner_type)
>>  {
>>   /* Do the following before stripping toplevel qualifiers.  */
>>   if (POINTER_TYPE_P (inner_type)
>>       && POINTER_TYPE_P (outer_type))
>>     {
>> +      /* Do not lose casts between pointers of different precision.  */
>> +      if (TYPE_PRECISION (outer_type)
>> +         != TYPE_PRECISION (inner_type))
>> +       return false;
>> +
>>       /* Do not lose casts between pointers to different address spaces.  */
>>       if (TYPE_ADDR_SPACE (TREE_TYPE (outer_type))
>>          != TYPE_ADDR_SPACE (TREE_TYPE (inner_type)))
>>        return false;
>> 
>>       /* If the outer type is (void *), the conversion is not necessary.  */
> 
> That should not be necessary as there is a mode check below.  Do you
> hit the issue only when the VOID_TYPE_P check is true?  In that case
> simply delete it - it has become obsolete.

We hit the same issue for VMS while compiling libada.  I just checked that removing the check fixes the crash.

Just for the record.

Tristan.
DJ Delorie Jan. 13, 2012, 9:59 p.m. UTC | #3
> That should not be necessary as there is a mode check below.  Do you
> hit the issue only when the VOID_TYPE_P check is true?  In that case
> simply delete it - it has become obsolete.

That seems to be happening, yes, but there are other checks
that might let differing modes through...

  /* Changes in machine mode are never useless conversions unless we
     deal with aggregate types in which case we defer to later checks.  */
  if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type)
      && !AGGREGATE_TYPE_P (inner_type))
    return false;

Later,

  else if (POINTER_TYPE_P (inner_type)
	   && POINTER_TYPE_P (outer_type))
    {
      /* Do not lose casts to function pointer types.  */
      if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
	   || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
	  && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE
	       || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE))
	return false;

      /* We do not care for const qualification of the pointed-to types
	 as const qualification has no semantic value to the middle-end.  */

      /* Otherwise pointers/references are equivalent.  */
      return true;
    }

So two different sized pointers to aggregate types will also have a problem?

I'm a little paranoid here because TPF uses different sized pointers a
*lot* so if we can't guarantee that the rest of the logic keeps the
conversion, I'd rather keep the explicit check.
Eric Botcazou Jan. 15, 2012, 9:12 p.m. UTC | #4
> So two different sized pointers to aggregate types will also have a
> problem?

Nope, you misread the test:

  /* Changes in machine mode are never useless conversions unless we
     deal with aggregate types in which case we defer to later checks.  */
  if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type)
      && !AGGREGATE_TYPE_P (inner_type))
    return false;

This will return false for any couple of pointers with different modes.
Tristan Gingold Jan. 16, 2012, 9:10 a.m. UTC | #5
On Jan 13, 2012, at 10:59 PM, DJ Delorie wrote:

> 
>> That should not be necessary as there is a mode check below.  Do you
>> hit the issue only when the VOID_TYPE_P check is true?  In that case
>> simply delete it - it has become obsolete.
> 
> That seems to be happening, yes, but there are other checks
> that might let differing modes through...
> 
>  /* Changes in machine mode are never useless conversions unless we
>     deal with aggregate types in which case we defer to later checks.  */
>  if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type)
>      && !AGGREGATE_TYPE_P (inner_type))
>    return false;
> 
> Later,
> 
>  else if (POINTER_TYPE_P (inner_type)
> 	   && POINTER_TYPE_P (outer_type))
>    {
>      /* Do not lose casts to function pointer types.  */
>      if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
> 	   || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
> 	  && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE
> 	       || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE))
> 	return false;
> 
>      /* We do not care for const qualification of the pointed-to types
> 	 as const qualification has no semantic value to the middle-end.  */
> 
>      /* Otherwise pointers/references are equivalent.  */
>      return true;
>    }
> 
> So two different sized pointers to aggregate types will also have a problem?

No, because pointers are not aggregate types.

> 
> I'm a little paranoid here because TPF uses different sized pointers a
> *lot* so if we can't guarantee that the rest of the logic keeps the
> conversion, I'd rather keep the explicit check.

For the record, what is TPF ?

Tristan.
DJ Delorie Jan. 16, 2012, 7:27 p.m. UTC | #6
Ah!  I read that as "pointer to aggregate" :-P
DJ Delorie Jan. 16, 2012, 7:29 p.m. UTC | #7
> For the record, what is TPF ?

One of IBM's s390x operating systems.  ../gcc/configure --target=tpf
diff mbox

Patch

Index: tree-ssa.c
===================================================================
--- tree-ssa.c	(revision 183139)
+++ tree-ssa.c	(working copy)
@@ -1192,12 +1192,17 @@  bool
 useless_type_conversion_p (tree outer_type, tree inner_type)
 {
   /* Do the following before stripping toplevel qualifiers.  */
   if (POINTER_TYPE_P (inner_type)
       && POINTER_TYPE_P (outer_type))
     {
+      /* Do not lose casts between pointers of different precision.  */
+      if (TYPE_PRECISION (outer_type)
+	  != TYPE_PRECISION (inner_type))
+	return false;
+
       /* Do not lose casts between pointers to different address spaces.  */
       if (TYPE_ADDR_SPACE (TREE_TYPE (outer_type))
 	  != TYPE_ADDR_SPACE (TREE_TYPE (inner_type)))
 	return false;
 
       /* If the outer type is (void *), the conversion is not necessary.  */