Refactor gimple_expr_type

Refactor gimple_expr_type

[Aditya K]

Hi,
I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if.

Please review this patch.

Thanks,
-Aditya


gcc/ChangeLog:

2015-05-15  hiraditya  <hiraditya@msn.com>

        * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.

diff --git a/gcc/gimple.h b/gcc/gimple.h
index 95e4fc8..168d3ba 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -5717,35 +5717,28 @@ static inline tree
 gimple_expr_type (const_gimple stmt)
 {
   enum gimple_code code = gimple_code (stmt);
-
-  if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
+  tree type;
+  /* In general we want to pass out a type that can be substituted
+     for both the RHS and the LHS types if there is a possibly
+     useless conversion involved.  That means returning the
+     original RHS type as far as we can reconstruct it.  */
+  if (code == GIMPLE_CALL)
     {
-      tree type;
-      /* In general we want to pass out a type that can be substituted
-         for both the RHS and the LHS types if there is a possibly
-        useless conversion involved.  That means returning the
-        original RHS type as far as we can reconstruct it.  */
-      if (code == GIMPLE_CALL)
-       {
-         const gcall *call_stmt = as_a <const gcall *> (stmt);
-         if (gimple_call_internal_p (call_stmt)
-             && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
-           type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
-         else
-           type = gimple_call_return_type (call_stmt);
-       }
+      const gcall *call_stmt = as_a <const gcall *> (stmt);
+      if (gimple_call_internal_p (call_stmt)
+          && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
+        type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
+      else
+        type = gimple_call_return_type (call_stmt);
+      return type;
+    }
+  else if (code == GIMPLE_ASSIGN)
+    {
+      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
+        type = TREE_TYPE (gimple_assign_rhs1 (stmt));
       else
-       switch (gimple_assign_rhs_code (stmt))
-         {
-         case POINTER_PLUS_EXPR:
-           type = TREE_TYPE (gimple_assign_rhs1 (stmt));
-           break;
-
-         default:
-           /* As fallback use the type of the LHS.  */
-           type = TREE_TYPE (gimple_get_lhs (stmt));
-           break;
-         }
+        /* As fallback use the type of the LHS.  */
+        type = TREE_TYPE (gimple_get_lhs (stmt));
       return type;
     }
   else if (code == GIMPLE_COND)

 		 	   		  

Re: Refactor gimple_expr_type

[Trevor Saunders]

On Fri, May 15, 2015 at 07:13:35AM +0000, Aditya K wrote:
> Hi,
> I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if.
> 
> Please review this patch.

for some reason your mail client seems to be inserting non breaking
spaces all over the place.  Please either configure it to not do that,
or use git send-email for patches.

> 
> Thanks,
> -Aditya
> 
> 
> gcc/ChangeLog:
> 
> 2015-05-15  hiraditya  <hiraditya@msn.com>
> 
>         * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
> 
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index 95e4fc8..168d3ba 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -5717,35 +5717,28 @@ static inline tree
>  gimple_expr_type (const_gimple stmt)
>  {
>    enum gimple_code code = gimple_code (stmt);
> -
> -  if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
> +  tree type;
> +  /* In general we want to pass out a type that can be substituted
> +     for both the RHS and the LHS types if there is a possibly
> +     useless conversion involved.  That means returning the
> +     original RHS type as far as we can reconstruct it.  */
> +  if (code == GIMPLE_CALL)
>      {
> -      tree type;
> -      /* In general we want to pass out a type that can be substituted
> -         for both the RHS and the LHS types if there is a possibly
> -        useless conversion involved.  That means returning the
> -        original RHS type as far as we can reconstruct it.  */
> -      if (code == GIMPLE_CALL)
> -       {
> -         const gcall *call_stmt = as_a <const gcall *> (stmt);
> -         if (gimple_call_internal_p (call_stmt)
> -             && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
> -           type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
> -         else
> -           type = gimple_call_return_type (call_stmt);
> -       }
> +      const gcall *call_stmt = as_a <const gcall *> (stmt);
> +      if (gimple_call_internal_p (call_stmt)
> +          && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
> +        type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
> +      else
> +        type = gimple_call_return_type (call_stmt);
> +      return type;

You might as well return the value directly and not use the variable.

> +    }
> +  else if (code == GIMPLE_ASSIGN)

else after return is kind of silly.

> +    {
> +      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
> +        type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>        else
> -       switch (gimple_assign_rhs_code (stmt))
> -         {
> -         case POINTER_PLUS_EXPR:
> -           type = TREE_TYPE (gimple_assign_rhs1 (stmt));
> -           break;
> -
> -         default:
> -           /* As fallback use the type of the LHS.  */
> -           type = TREE_TYPE (gimple_get_lhs (stmt));
> -           break;
> -         }
> +        /* As fallback use the type of the LHS.  */
> +        type = TREE_TYPE (gimple_get_lhs (stmt));
>        return type;

might as well not use type here either.

Trev

>      }
>    else if (code == GIMPLE_COND)
> 
>  		 	   		  

RE: Refactor gimple_expr_type

[Aditya K]

----------------------------------------
> Date: Sat, 16 May 2015 11:53:57 -0400
> From: tbsaunde@tbsaunde.org
> To: hiraditya@msn.com
> CC: gcc-patches@gcc.gnu.org
> Subject: Re: Refactor gimple_expr_type
>
> On Fri, May 15, 2015 at 07:13:35AM +0000, Aditya K wrote:
>> Hi,
>> I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if.
>>
>> Please review this patch.
>
> for some reason your mail client seems to be inserting non breaking
> spaces all over the place. Please either configure it to not do that,
> or use git send-email for patches.

Please see the updated patch.
gcc/ChangeLog:

2015-05-15  hiraditya  <hiraditya@msn.com>

        * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.

diff --git a/gcc/gimple.h b/gcc/gimple.h
index 95e4fc8..3a83e8f 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -5717,36 +5717,26 @@ static inline tree
 gimple_expr_type (const_gimple stmt)
 {
   enum gimple_code code = gimple_code (stmt);
-
-  if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
+  /* In general we want to pass out a type that can be substituted
+     for both the RHS and the LHS types if there is a possibly
+     useless conversion involved.  That means returning the
+     original RHS type as far as we can reconstruct it.  */
+  if (code == GIMPLE_CALL)
     {
-      tree type;
-      /* In general we want to pass out a type that can be substituted
-         for both the RHS and the LHS types if there is a possibly
-        useless conversion involved.  That means returning the
-        original RHS type as far as we can reconstruct it.  */
-      if (code == GIMPLE_CALL)
-       {
-         const gcall *call_stmt = as_a <const gcall *> (stmt);
-         if (gimple_call_internal_p (call_stmt)
-             && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
-           type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
-         else
-           type = gimple_call_return_type (call_stmt);
-       }
+      const gcall *call_stmt = as_a <const gcall *> (stmt);
+      if (gimple_call_internal_p (call_stmt)
+          && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
+        return TREE_TYPE (gimple_call_arg (call_stmt, 3));
+      else
+        return gimple_call_return_type (call_stmt);
+    }
+  else if (code == GIMPLE_ASSIGN)
+    {
+      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
+        return TREE_TYPE (gimple_assign_rhs1 (stmt));
       else
-       switch (gimple_assign_rhs_code (stmt))
-         {
-         case POINTER_PLUS_EXPR:
-           type = TREE_TYPE (gimple_assign_rhs1 (stmt));
-           break;
-
-         default:
-           /* As fallback use the type of the LHS.  */
-           type = TREE_TYPE (gimple_get_lhs (stmt));
-           break;
-         }
-      return type;
+        /* As fallback use the type of the LHS.  */
+        return TREE_TYPE (gimple_get_lhs (stmt));
     }
   else if (code == GIMPLE_COND)
     return boolean_type_node;


Thanks,
-Aditya





>
>>
>> Thanks,
>> -Aditya
>>
>>
>> gcc/ChangeLog:
>>
>> 2015-05-15  hiraditya  <hiraditya@msn.com>
>>
>>         * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>
>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>> index 95e4fc8..168d3ba 100644
>> --- a/gcc/gimple.h
>> +++ b/gcc/gimple.h
>> @@ -5717,35 +5717,28 @@ static inline tree
>>  gimple_expr_type (const_gimple stmt)
>>  {
>>    enum gimple_code code = gimple_code (stmt);
>> -
>> -  if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>> +  tree type;
>> +  /* In general we want to pass out a type that can be substituted
>> +     for both the RHS and the LHS types if there is a possibly
>> +     useless conversion involved.  That means returning the
>> +     original RHS type as far as we can reconstruct it.  */
>> +  if (code == GIMPLE_CALL)
>>      {
>> -      tree type;
>> -      /* In general we want to pass out a type that can be substituted
>> -         for both the RHS and the LHS types if there is a possibly
>> -        useless conversion involved.  That means returning the
>> -        original RHS type as far as we can reconstruct it.  */
>> -      if (code == GIMPLE_CALL)
>> -       {
>> -         const gcall *call_stmt = as_a <const gcall *> (stmt);
>> -         if (gimple_call_internal_p (call_stmt)
>> -             && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>> -           type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>> -         else
>> -           type = gimple_call_return_type (call_stmt);
>> -       }
>> +      const gcall *call_stmt = as_a <const gcall *> (stmt);
>> +      if (gimple_call_internal_p (call_stmt)
>> +          && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>> +        type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>> +      else
>> +        type = gimple_call_return_type (call_stmt);
>> +      return type;
>
> You might as well return the value directly and not use the variable.
>
>> +    }
>> +  else if (code == GIMPLE_ASSIGN)
>
> else after return is kind of silly.
>
>> +    {
>> +      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>> +        type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>        else
>> -       switch (gimple_assign_rhs_code (stmt))
>> -         {
>> -         case POINTER_PLUS_EXPR:
>> -           type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>> -           break;
>> -
>> -         default:
>> -           /* As fallback use the type of the LHS.  */
>> -           type = TREE_TYPE (gimple_get_lhs (stmt));
>> -           break;
>> -         }
>> +        /* As fallback use the type of the LHS.  */
>> +        type = TREE_TYPE (gimple_get_lhs (stmt));
>>        return type;
>
> might as well not use type here either.
>
> Trev
>
>>      }
>>    else if (code == GIMPLE_COND)
>>
>>

 		 	   		  

Re: Refactor gimple_expr_type

[Richard Biener]

On Sun, May 17, 2015 at 5:31 PM, Aditya K <hiraditya@msn.com> wrote:
>
>
> ----------------------------------------
>> Date: Sat, 16 May 2015 11:53:57 -0400
>> From: tbsaunde@tbsaunde.org
>> To: hiraditya@msn.com
>> CC: gcc-patches@gcc.gnu.org
>> Subject: Re: Refactor gimple_expr_type
>>
>> On Fri, May 15, 2015 at 07:13:35AM +0000, Aditya K wrote:
>>> Hi,
>>> I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if.
>>>
>>> Please review this patch.
>>
>> for some reason your mail client seems to be inserting non breaking
>> spaces all over the place. Please either configure it to not do that,
>> or use git send-email for patches.
>
> Please see the updated patch.

Ok if this passed bootstrap and regtest.  (I wish if gimple_expr_type
didn't exist btw...)

Thanks,
Richard.

> gcc/ChangeLog:
>
> 2015-05-15  hiraditya  <hiraditya@msn.com>
>
>         * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index 95e4fc8..3a83e8f 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -5717,36 +5717,26 @@ static inline tree
>  gimple_expr_type (const_gimple stmt)
>  {
>    enum gimple_code code = gimple_code (stmt);
> -
> -  if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
> +  /* In general we want to pass out a type that can be substituted
> +     for both the RHS and the LHS types if there is a possibly
> +     useless conversion involved.  That means returning the
> +     original RHS type as far as we can reconstruct it.  */
> +  if (code == GIMPLE_CALL)
>      {
> -      tree type;
> -      /* In general we want to pass out a type that can be substituted
> -         for both the RHS and the LHS types if there is a possibly
> -        useless conversion involved.  That means returning the
> -        original RHS type as far as we can reconstruct it.  */
> -      if (code == GIMPLE_CALL)
> -       {
> -         const gcall *call_stmt = as_a <const gcall *> (stmt);
> -         if (gimple_call_internal_p (call_stmt)
> -             && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
> -           type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
> -         else
> -           type = gimple_call_return_type (call_stmt);
> -       }
> +      const gcall *call_stmt = as_a <const gcall *> (stmt);
> +      if (gimple_call_internal_p (call_stmt)
> +          && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
> +        return TREE_TYPE (gimple_call_arg (call_stmt, 3));
> +      else
> +        return gimple_call_return_type (call_stmt);
> +    }
> +  else if (code == GIMPLE_ASSIGN)
> +    {
> +      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
> +        return TREE_TYPE (gimple_assign_rhs1 (stmt));
>        else
> -       switch (gimple_assign_rhs_code (stmt))
> -         {
> -         case POINTER_PLUS_EXPR:
> -           type = TREE_TYPE (gimple_assign_rhs1 (stmt));
> -           break;
> -
> -         default:
> -           /* As fallback use the type of the LHS.  */
> -           type = TREE_TYPE (gimple_get_lhs (stmt));
> -           break;
> -         }
> -      return type;
> +        /* As fallback use the type of the LHS.  */
> +        return TREE_TYPE (gimple_get_lhs (stmt));
>      }
>    else if (code == GIMPLE_COND)
>      return boolean_type_node;
>
>
> Thanks,
> -Aditya
>
>
>
>
>
>>
>>>
>>> Thanks,
>>> -Aditya
>>>
>>>
>>> gcc/ChangeLog:
>>>
>>> 2015-05-15  hiraditya  <hiraditya@msn.com>
>>>
>>>         * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>
>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>> index 95e4fc8..168d3ba 100644
>>> --- a/gcc/gimple.h
>>> +++ b/gcc/gimple.h
>>> @@ -5717,35 +5717,28 @@ static inline tree
>>>  gimple_expr_type (const_gimple stmt)
>>>  {
>>>    enum gimple_code code = gimple_code (stmt);
>>> -
>>> -  if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>> +  tree type;
>>> +  /* In general we want to pass out a type that can be substituted
>>> +     for both the RHS and the LHS types if there is a possibly
>>> +     useless conversion involved.  That means returning the
>>> +     original RHS type as far as we can reconstruct it.  */
>>> +  if (code == GIMPLE_CALL)
>>>      {
>>> -      tree type;
>>> -      /* In general we want to pass out a type that can be substituted
>>> -         for both the RHS and the LHS types if there is a possibly
>>> -        useless conversion involved.  That means returning the
>>> -        original RHS type as far as we can reconstruct it.  */
>>> -      if (code == GIMPLE_CALL)
>>> -       {
>>> -         const gcall *call_stmt = as_a <const gcall *> (stmt);
>>> -         if (gimple_call_internal_p (call_stmt)
>>> -             && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>> -           type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>> -         else
>>> -           type = gimple_call_return_type (call_stmt);
>>> -       }
>>> +      const gcall *call_stmt = as_a <const gcall *> (stmt);
>>> +      if (gimple_call_internal_p (call_stmt)
>>> +          && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>> +        type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>> +      else
>>> +        type = gimple_call_return_type (call_stmt);
>>> +      return type;
>>
>> You might as well return the value directly and not use the variable.
>>
>>> +    }
>>> +  else if (code == GIMPLE_ASSIGN)
>>
>> else after return is kind of silly.
>>
>>> +    {
>>> +      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>> +        type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>        else
>>> -       switch (gimple_assign_rhs_code (stmt))
>>> -         {
>>> -         case POINTER_PLUS_EXPR:
>>> -           type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>> -           break;
>>> -
>>> -         default:
>>> -           /* As fallback use the type of the LHS.  */
>>> -           type = TREE_TYPE (gimple_get_lhs (stmt));
>>> -           break;
>>> -         }
>>> +        /* As fallback use the type of the LHS.  */
>>> +        type = TREE_TYPE (gimple_get_lhs (stmt));
>>>        return type;
>>
>> might as well not use type here either.
>>
>> Trev
>>
>>>      }
>>>    else if (code == GIMPLE_COND)
>>>
>>>
>
>

RE: Refactor gimple_expr_type

[Aditya K]

----------------------------------------
> Date: Mon, 18 May 2015 12:08:58 +0200
> Subject: Re: Refactor gimple_expr_type
> From: richard.guenther@gmail.com
> To: hiraditya@msn.com
> CC: tbsaunde@tbsaunde.org; gcc-patches@gcc.gnu.org
>
> On Sun, May 17, 2015 at 5:31 PM, Aditya K <hiraditya@msn.com> wrote:
>>
>>
>> ----------------------------------------
>>> Date: Sat, 16 May 2015 11:53:57 -0400
>>> From: tbsaunde@tbsaunde.org
>>> To: hiraditya@msn.com
>>> CC: gcc-patches@gcc.gnu.org
>>> Subject: Re: Refactor gimple_expr_type
>>>
>>> On Fri, May 15, 2015 at 07:13:35AM +0000, Aditya K wrote:
>>>> Hi,
>>>> I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if.
>>>>
>>>> Please review this patch.
>>>
>>> for some reason your mail client seems to be inserting non breaking
>>> spaces all over the place. Please either configure it to not do that,
>>> or use git send-email for patches.
>>
>> Please see the updated patch.
>
> Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type
> didn't exist btw...)

Thanks for the review. Do you have any suggestions on how to remove gimple_expr_type. Are there any alternatives to it?
I can look into refactoring more (if it is not too complicated) since I'm already doing this.

-Aditya

>
> Thanks,
> Richard.
>
>> gcc/ChangeLog:
>>
>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>
>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>
>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>> index 95e4fc8..3a83e8f 100644
>> --- a/gcc/gimple.h
>> +++ b/gcc/gimple.h
>> @@ -5717,36 +5717,26 @@ static inline tree
>> gimple_expr_type (const_gimple stmt)
>> {
>> enum gimple_code code = gimple_code (stmt);
>> -
>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>> + /* In general we want to pass out a type that can be substituted
>> + for both the RHS and the LHS types if there is a possibly
>> + useless conversion involved. That means returning the
>> + original RHS type as far as we can reconstruct it. */
>> + if (code == GIMPLE_CALL)
>> {
>> - tree type;
>> - /* In general we want to pass out a type that can be substituted
>> - for both the RHS and the LHS types if there is a possibly
>> - useless conversion involved. That means returning the
>> - original RHS type as far as we can reconstruct it. */
>> - if (code == GIMPLE_CALL)
>> - {
>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>> - if (gimple_call_internal_p (call_stmt)
>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>> - else
>> - type = gimple_call_return_type (call_stmt);
>> - }
>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>> + if (gimple_call_internal_p (call_stmt)
>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>> + return TREE_TYPE (gimple_call_arg (call_stmt, 3));
>> + else
>> + return gimple_call_return_type (call_stmt);
>> + }
>> + else if (code == GIMPLE_ASSIGN)
>> + {
>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>> + return TREE_TYPE (gimple_assign_rhs1 (stmt));
>> else
>> - switch (gimple_assign_rhs_code (stmt))
>> - {
>> - case POINTER_PLUS_EXPR:
>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>> - break;
>> -
>> - default:
>> - /* As fallback use the type of the LHS. */
>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>> - break;
>> - }
>> - return type;
>> + /* As fallback use the type of the LHS. */
>> + return TREE_TYPE (gimple_get_lhs (stmt));
>> }
>> else if (code == GIMPLE_COND)
>> return boolean_type_node;
>>
>>
>> Thanks,
>> -Aditya
>>
>>
>>
>>
>>
>>>
>>>>
>>>> Thanks,
>>>> -Aditya
>>>>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>>>
>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>>
>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>> index 95e4fc8..168d3ba 100644
>>>> --- a/gcc/gimple.h
>>>> +++ b/gcc/gimple.h
>>>> @@ -5717,35 +5717,28 @@ static inline tree
>>>> gimple_expr_type (const_gimple stmt)
>>>> {
>>>> enum gimple_code code = gimple_code (stmt);
>>>> -
>>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>>> + tree type;
>>>> + /* In general we want to pass out a type that can be substituted
>>>> + for both the RHS and the LHS types if there is a possibly
>>>> + useless conversion involved. That means returning the
>>>> + original RHS type as far as we can reconstruct it. */
>>>> + if (code == GIMPLE_CALL)
>>>> {
>>>> - tree type;
>>>> - /* In general we want to pass out a type that can be substituted
>>>> - for both the RHS and the LHS types if there is a possibly
>>>> - useless conversion involved. That means returning the
>>>> - original RHS type as far as we can reconstruct it. */
>>>> - if (code == GIMPLE_CALL)
>>>> - {
>>>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>> - if (gimple_call_internal_p (call_stmt)
>>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>> - else
>>>> - type = gimple_call_return_type (call_stmt);
>>>> - }
>>>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>> + if (gimple_call_internal_p (call_stmt)
>>>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>> + type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>> + else
>>>> + type = gimple_call_return_type (call_stmt);
>>>> + return type;
>>>
>>> You might as well return the value directly and not use the variable.
>>>
>>>> + }
>>>> + else if (code == GIMPLE_ASSIGN)
>>>
>>> else after return is kind of silly.
>>>
>>>> + {
>>>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>>> + type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>> else
>>>> - switch (gimple_assign_rhs_code (stmt))
>>>> - {
>>>> - case POINTER_PLUS_EXPR:
>>>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>> - break;
>>>> -
>>>> - default:
>>>> - /* As fallback use the type of the LHS. */
>>>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>>>> - break;
>>>> - }
>>>> + /* As fallback use the type of the LHS. */
>>>> + type = TREE_TYPE (gimple_get_lhs (stmt));
>>>> return type;
>>>
>>> might as well not use type here either.
>>>
>>> Trev
>>>
>>>> }
>>>> else if (code == GIMPLE_COND)
>>>>
>>>>
>>
>>
 		 	   		  

Re: Refactor gimple_expr_type

[Richard Biener]

On Tue, May 19, 2015 at 12:04 AM, Aditya K <hiraditya@msn.com> wrote:
>
>
> ----------------------------------------
>> Date: Mon, 18 May 2015 12:08:58 +0200
>> Subject: Re: Refactor gimple_expr_type
>> From: richard.guenther@gmail.com
>> To: hiraditya@msn.com
>> CC: tbsaunde@tbsaunde.org; gcc-patches@gcc.gnu.org
>>
>> On Sun, May 17, 2015 at 5:31 PM, Aditya K <hiraditya@msn.com> wrote:
>>>
>>>
>>> ----------------------------------------
>>>> Date: Sat, 16 May 2015 11:53:57 -0400
>>>> From: tbsaunde@tbsaunde.org
>>>> To: hiraditya@msn.com
>>>> CC: gcc-patches@gcc.gnu.org
>>>> Subject: Re: Refactor gimple_expr_type
>>>>
>>>> On Fri, May 15, 2015 at 07:13:35AM +0000, Aditya K wrote:
>>>>> Hi,
>>>>> I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if.
>>>>>
>>>>> Please review this patch.
>>>>
>>>> for some reason your mail client seems to be inserting non breaking
>>>> spaces all over the place. Please either configure it to not do that,
>>>> or use git send-email for patches.
>>>
>>> Please see the updated patch.
>>
>> Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type
>> didn't exist btw...)
>
> Thanks for the review. Do you have any suggestions on how to remove gimple_expr_type. Are there any alternatives to it?
> I can look into refactoring more (if it is not too complicated) since I'm already doing this.

Look at each caller - usually they should be fine with using TREE_TYPE
(gimple_get_lhs ()) (or a more specific one
dependent on what stmts are expected at the place).  You might want to
first refactor the code

  else if (code == GIMPLE_COND)
    gcc_unreachable ();

and deal with the fallout in callers (similar for the void_type_node return).

Richard.


> -Aditya
>
>>
>> Thanks,
>> Richard.
>>
>>> gcc/ChangeLog:
>>>
>>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>>
>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>
>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>> index 95e4fc8..3a83e8f 100644
>>> --- a/gcc/gimple.h
>>> +++ b/gcc/gimple.h
>>> @@ -5717,36 +5717,26 @@ static inline tree
>>> gimple_expr_type (const_gimple stmt)
>>> {
>>> enum gimple_code code = gimple_code (stmt);
>>> -
>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>> + /* In general we want to pass out a type that can be substituted
>>> + for both the RHS and the LHS types if there is a possibly
>>> + useless conversion involved. That means returning the
>>> + original RHS type as far as we can reconstruct it. */
>>> + if (code == GIMPLE_CALL)
>>> {
>>> - tree type;
>>> - /* In general we want to pass out a type that can be substituted
>>> - for both the RHS and the LHS types if there is a possibly
>>> - useless conversion involved. That means returning the
>>> - original RHS type as far as we can reconstruct it. */
>>> - if (code == GIMPLE_CALL)
>>> - {
>>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>>> - if (gimple_call_internal_p (call_stmt)
>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>> - else
>>> - type = gimple_call_return_type (call_stmt);
>>> - }
>>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>>> + if (gimple_call_internal_p (call_stmt)
>>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>> + return TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>> + else
>>> + return gimple_call_return_type (call_stmt);
>>> + }
>>> + else if (code == GIMPLE_ASSIGN)
>>> + {
>>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>> + return TREE_TYPE (gimple_assign_rhs1 (stmt));
>>> else
>>> - switch (gimple_assign_rhs_code (stmt))
>>> - {
>>> - case POINTER_PLUS_EXPR:
>>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>> - break;
>>> -
>>> - default:
>>> - /* As fallback use the type of the LHS. */
>>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>>> - break;
>>> - }
>>> - return type;
>>> + /* As fallback use the type of the LHS. */
>>> + return TREE_TYPE (gimple_get_lhs (stmt));
>>> }
>>> else if (code == GIMPLE_COND)
>>> return boolean_type_node;
>>>
>>>
>>> Thanks,
>>> -Aditya
>>>
>>>
>>>
>>>
>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> -Aditya
>>>>>
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>>>>
>>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>>>
>>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>>> index 95e4fc8..168d3ba 100644
>>>>> --- a/gcc/gimple.h
>>>>> +++ b/gcc/gimple.h
>>>>> @@ -5717,35 +5717,28 @@ static inline tree
>>>>> gimple_expr_type (const_gimple stmt)
>>>>> {
>>>>> enum gimple_code code = gimple_code (stmt);
>>>>> -
>>>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>>>> + tree type;
>>>>> + /* In general we want to pass out a type that can be substituted
>>>>> + for both the RHS and the LHS types if there is a possibly
>>>>> + useless conversion involved. That means returning the
>>>>> + original RHS type as far as we can reconstruct it. */
>>>>> + if (code == GIMPLE_CALL)
>>>>> {
>>>>> - tree type;
>>>>> - /* In general we want to pass out a type that can be substituted
>>>>> - for both the RHS and the LHS types if there is a possibly
>>>>> - useless conversion involved. That means returning the
>>>>> - original RHS type as far as we can reconstruct it. */
>>>>> - if (code == GIMPLE_CALL)
>>>>> - {
>>>>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>> - if (gimple_call_internal_p (call_stmt)
>>>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>> - else
>>>>> - type = gimple_call_return_type (call_stmt);
>>>>> - }
>>>>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>> + if (gimple_call_internal_p (call_stmt)
>>>>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>> + type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>> + else
>>>>> + type = gimple_call_return_type (call_stmt);
>>>>> + return type;
>>>>
>>>> You might as well return the value directly and not use the variable.
>>>>
>>>>> + }
>>>>> + else if (code == GIMPLE_ASSIGN)
>>>>
>>>> else after return is kind of silly.
>>>>
>>>>> + {
>>>>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>>>> + type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>> else
>>>>> - switch (gimple_assign_rhs_code (stmt))
>>>>> - {
>>>>> - case POINTER_PLUS_EXPR:
>>>>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>> - break;
>>>>> -
>>>>> - default:
>>>>> - /* As fallback use the type of the LHS. */
>>>>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>> - break;
>>>>> - }
>>>>> + /* As fallback use the type of the LHS. */
>>>>> + type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>> return type;
>>>>
>>>> might as well not use type here either.
>>>>
>>>> Trev
>>>>
>>>>> }
>>>>> else if (code == GIMPLE_COND)
>>>>>
>>>>>
>>>
>>>
>

RE: Refactor gimple_expr_type

[Aditya K]

----------------------------------------
> Date: Tue, 19 May 2015 11:33:16 +0200
> Subject: Re: Refactor gimple_expr_type
> From: richard.guenther@gmail.com
> To: hiraditya@msn.com
> CC: tbsaunde@tbsaunde.org; gcc-patches@gcc.gnu.org
>
> On Tue, May 19, 2015 at 12:04 AM, Aditya K <hiraditya@msn.com> wrote:
>>
>>
>> ----------------------------------------
>>> Date: Mon, 18 May 2015 12:08:58 +0200
>>> Subject: Re: Refactor gimple_expr_type
>>> From: richard.guenther@gmail.com
>>> To: hiraditya@msn.com
>>> CC: tbsaunde@tbsaunde.org; gcc-patches@gcc.gnu.org
>>>
>>> On Sun, May 17, 2015 at 5:31 PM, Aditya K <hiraditya@msn.com> wrote:
>>>>
>>>>
>>>> ----------------------------------------
>>>>> Date: Sat, 16 May 2015 11:53:57 -0400
>>>>> From: tbsaunde@tbsaunde.org
>>>>> To: hiraditya@msn.com
>>>>> CC: gcc-patches@gcc.gnu.org
>>>>> Subject: Re: Refactor gimple_expr_type
>>>>>
>>>>> On Fri, May 15, 2015 at 07:13:35AM +0000, Aditya K wrote:
>>>>>> Hi,
>>>>>> I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if.
>>>>>>
>>>>>> Please review this patch.
>>>>>
>>>>> for some reason your mail client seems to be inserting non breaking
>>>>> spaces all over the place. Please either configure it to not do that,
>>>>> or use git send-email for patches.
>>>>
>>>> Please see the updated patch.
>>>
>>> Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type
>>> didn't exist btw...)
>>
>> Thanks for the review. Do you have any suggestions on how to remove gimple_expr_type. Are there any alternatives to it?
>> I can look into refactoring more (if it is not too complicated) since I'm already doing this.
>
> Look at each caller - usually they should be fine with using TREE_TYPE
> (gimple_get_lhs ()) (or a more specific one
> dependent on what stmts are expected at the place). You might want to
> first refactor the code
>
> else if (code == GIMPLE_COND)
> gcc_unreachable ();
>
> and deal with the fallout in callers (similar for the void_type_node return).

Thanks for the suggestions. I looked at the use cases there are 47 usages in different files. That might be a lot of changes I assume, and would take some time.
This patch passes bootstrap and make check (although I'm not very confident that my way of make check ran all the regtests)

If this patch is okay to merge please do that. I'll continue working on removing gimle_expr_type.

Thanks,
-Aditya


>
> Richard.
>
>
>> -Aditya
>>
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>>>
>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>>
>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>> index 95e4fc8..3a83e8f 100644
>>>> --- a/gcc/gimple.h
>>>> +++ b/gcc/gimple.h
>>>> @@ -5717,36 +5717,26 @@ static inline tree
>>>> gimple_expr_type (const_gimple stmt)
>>>> {
>>>> enum gimple_code code = gimple_code (stmt);
>>>> -
>>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>>> + /* In general we want to pass out a type that can be substituted
>>>> + for both the RHS and the LHS types if there is a possibly
>>>> + useless conversion involved. That means returning the
>>>> + original RHS type as far as we can reconstruct it. */
>>>> + if (code == GIMPLE_CALL)
>>>> {
>>>> - tree type;
>>>> - /* In general we want to pass out a type that can be substituted
>>>> - for both the RHS and the LHS types if there is a possibly
>>>> - useless conversion involved. That means returning the
>>>> - original RHS type as far as we can reconstruct it. */
>>>> - if (code == GIMPLE_CALL)
>>>> - {
>>>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>> - if (gimple_call_internal_p (call_stmt)
>>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>> - else
>>>> - type = gimple_call_return_type (call_stmt);
>>>> - }
>>>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>> + if (gimple_call_internal_p (call_stmt)
>>>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>> + return TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>> + else
>>>> + return gimple_call_return_type (call_stmt);
>>>> + }
>>>> + else if (code == GIMPLE_ASSIGN)
>>>> + {
>>>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>>> + return TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>> else
>>>> - switch (gimple_assign_rhs_code (stmt))
>>>> - {
>>>> - case POINTER_PLUS_EXPR:
>>>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>> - break;
>>>> -
>>>> - default:
>>>> - /* As fallback use the type of the LHS. */
>>>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>>>> - break;
>>>> - }
>>>> - return type;
>>>> + /* As fallback use the type of the LHS. */
>>>> + return TREE_TYPE (gimple_get_lhs (stmt));
>>>> }
>>>> else if (code == GIMPLE_COND)
>>>> return boolean_type_node;
>>>>
>>>>
>>>> Thanks,
>>>> -Aditya
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> -Aditya
>>>>>>
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>>>>>
>>>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>>>>
>>>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>>>> index 95e4fc8..168d3ba 100644
>>>>>> --- a/gcc/gimple.h
>>>>>> +++ b/gcc/gimple.h
>>>>>> @@ -5717,35 +5717,28 @@ static inline tree
>>>>>> gimple_expr_type (const_gimple stmt)
>>>>>> {
>>>>>> enum gimple_code code = gimple_code (stmt);
>>>>>> -
>>>>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>>>>> + tree type;
>>>>>> + /* In general we want to pass out a type that can be substituted
>>>>>> + for both the RHS and the LHS types if there is a possibly
>>>>>> + useless conversion involved. That means returning the
>>>>>> + original RHS type as far as we can reconstruct it. */
>>>>>> + if (code == GIMPLE_CALL)
>>>>>> {
>>>>>> - tree type;
>>>>>> - /* In general we want to pass out a type that can be substituted
>>>>>> - for both the RHS and the LHS types if there is a possibly
>>>>>> - useless conversion involved. That means returning the
>>>>>> - original RHS type as far as we can reconstruct it. */
>>>>>> - if (code == GIMPLE_CALL)
>>>>>> - {
>>>>>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>>> - if (gimple_call_internal_p (call_stmt)
>>>>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>>> - else
>>>>>> - type = gimple_call_return_type (call_stmt);
>>>>>> - }
>>>>>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>>> + if (gimple_call_internal_p (call_stmt)
>>>>>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>>> + type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>>> + else
>>>>>> + type = gimple_call_return_type (call_stmt);
>>>>>> + return type;
>>>>>
>>>>> You might as well return the value directly and not use the variable.
>>>>>
>>>>>> + }
>>>>>> + else if (code == GIMPLE_ASSIGN)
>>>>>
>>>>> else after return is kind of silly.
>>>>>
>>>>>> + {
>>>>>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>>>>> + type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>>> else
>>>>>> - switch (gimple_assign_rhs_code (stmt))
>>>>>> - {
>>>>>> - case POINTER_PLUS_EXPR:
>>>>>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>>> - break;
>>>>>> -
>>>>>> - default:
>>>>>> - /* As fallback use the type of the LHS. */
>>>>>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>>> - break;
>>>>>> - }
>>>>>> + /* As fallback use the type of the LHS. */
>>>>>> + type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>>> return type;
>>>>>
>>>>> might as well not use type here either.
>>>>>
>>>>> Trev
>>>>>
>>>>>> }
>>>>>> else if (code == GIMPLE_COND)
>>>>>>
>>>>>>
>>>>
>>>>
>>


 		 	   		  

Re: Refactor gimple_expr_type

[Richard Biener]

On Tue, May 19, 2015 at 6:50 PM, Aditya K <hiraditya@msn.com> wrote:
>
>
> ----------------------------------------
>> Date: Tue, 19 May 2015 11:33:16 +0200
>> Subject: Re: Refactor gimple_expr_type
>> From: richard.guenther@gmail.com
>> To: hiraditya@msn.com
>> CC: tbsaunde@tbsaunde.org; gcc-patches@gcc.gnu.org
>>
>> On Tue, May 19, 2015 at 12:04 AM, Aditya K <hiraditya@msn.com> wrote:
>>>
>>>
>>> ----------------------------------------
>>>> Date: Mon, 18 May 2015 12:08:58 +0200
>>>> Subject: Re: Refactor gimple_expr_type
>>>> From: richard.guenther@gmail.com
>>>> To: hiraditya@msn.com
>>>> CC: tbsaunde@tbsaunde.org; gcc-patches@gcc.gnu.org
>>>>
>>>> On Sun, May 17, 2015 at 5:31 PM, Aditya K <hiraditya@msn.com> wrote:
>>>>>
>>>>>
>>>>> ----------------------------------------
>>>>>> Date: Sat, 16 May 2015 11:53:57 -0400
>>>>>> From: tbsaunde@tbsaunde.org
>>>>>> To: hiraditya@msn.com
>>>>>> CC: gcc-patches@gcc.gnu.org
>>>>>> Subject: Re: Refactor gimple_expr_type
>>>>>>
>>>>>> On Fri, May 15, 2015 at 07:13:35AM +0000, Aditya K wrote:
>>>>>>> Hi,
>>>>>>> I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if.
>>>>>>>
>>>>>>> Please review this patch.
>>>>>>
>>>>>> for some reason your mail client seems to be inserting non breaking
>>>>>> spaces all over the place. Please either configure it to not do that,
>>>>>> or use git send-email for patches.
>>>>>
>>>>> Please see the updated patch.
>>>>
>>>> Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type
>>>> didn't exist btw...)
>>>
>>> Thanks for the review. Do you have any suggestions on how to remove gimple_expr_type. Are there any alternatives to it?
>>> I can look into refactoring more (if it is not too complicated) since I'm already doing this.
>>
>> Look at each caller - usually they should be fine with using TREE_TYPE
>> (gimple_get_lhs ()) (or a more specific one
>> dependent on what stmts are expected at the place). You might want to
>> first refactor the code
>>
>> else if (code == GIMPLE_COND)
>> gcc_unreachable ();
>>
>> and deal with the fallout in callers (similar for the void_type_node return).
>
> Thanks for the suggestions. I looked at the use cases there are 47 usages in different files. That might be a lot of changes I assume, and would take some time.
> This patch passes bootstrap and make check (although I'm not very confident that my way of make check ran all the regtests)
>
> If this patch is okay to merge please do that. I'll continue working on removing gimle_expr_type.

Please re-send the patch as attachment, your mailer garbles the text
(send mails as non-unicode text/plain).

Richard.

> Thanks,
> -Aditya
>
>
>>
>> Richard.
>>
>>
>>> -Aditya
>>>
>>>>
>>>> Thanks,
>>>> Richard.
>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>>>>
>>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>>>
>>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>>> index 95e4fc8..3a83e8f 100644
>>>>> --- a/gcc/gimple.h
>>>>> +++ b/gcc/gimple.h
>>>>> @@ -5717,36 +5717,26 @@ static inline tree
>>>>> gimple_expr_type (const_gimple stmt)
>>>>> {
>>>>> enum gimple_code code = gimple_code (stmt);
>>>>> -
>>>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>>>> + /* In general we want to pass out a type that can be substituted
>>>>> + for both the RHS and the LHS types if there is a possibly
>>>>> + useless conversion involved. That means returning the
>>>>> + original RHS type as far as we can reconstruct it. */
>>>>> + if (code == GIMPLE_CALL)
>>>>> {
>>>>> - tree type;
>>>>> - /* In general we want to pass out a type that can be substituted
>>>>> - for both the RHS and the LHS types if there is a possibly
>>>>> - useless conversion involved. That means returning the
>>>>> - original RHS type as far as we can reconstruct it. */
>>>>> - if (code == GIMPLE_CALL)
>>>>> - {
>>>>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>> - if (gimple_call_internal_p (call_stmt)
>>>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>> - else
>>>>> - type = gimple_call_return_type (call_stmt);
>>>>> - }
>>>>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>> + if (gimple_call_internal_p (call_stmt)
>>>>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>> + return TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>> + else
>>>>> + return gimple_call_return_type (call_stmt);
>>>>> + }
>>>>> + else if (code == GIMPLE_ASSIGN)
>>>>> + {
>>>>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>>>> + return TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>> else
>>>>> - switch (gimple_assign_rhs_code (stmt))
>>>>> - {
>>>>> - case POINTER_PLUS_EXPR:
>>>>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>> - break;
>>>>> -
>>>>> - default:
>>>>> - /* As fallback use the type of the LHS. */
>>>>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>> - break;
>>>>> - }
>>>>> - return type;
>>>>> + /* As fallback use the type of the LHS. */
>>>>> + return TREE_TYPE (gimple_get_lhs (stmt));
>>>>> }
>>>>> else if (code == GIMPLE_COND)
>>>>> return boolean_type_node;
>>>>>
>>>>>
>>>>> Thanks,
>>>>> -Aditya
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> -Aditya
>>>>>>>
>>>>>>>
>>>>>>> gcc/ChangeLog:
>>>>>>>
>>>>>>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>>>>>>
>>>>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>>>>>
>>>>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>>>>> index 95e4fc8..168d3ba 100644
>>>>>>> --- a/gcc/gimple.h
>>>>>>> +++ b/gcc/gimple.h
>>>>>>> @@ -5717,35 +5717,28 @@ static inline tree
>>>>>>> gimple_expr_type (const_gimple stmt)
>>>>>>> {
>>>>>>> enum gimple_code code = gimple_code (stmt);
>>>>>>> -
>>>>>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>>>>>> + tree type;
>>>>>>> + /* In general we want to pass out a type that can be substituted
>>>>>>> + for both the RHS and the LHS types if there is a possibly
>>>>>>> + useless conversion involved. That means returning the
>>>>>>> + original RHS type as far as we can reconstruct it. */
>>>>>>> + if (code == GIMPLE_CALL)
>>>>>>> {
>>>>>>> - tree type;
>>>>>>> - /* In general we want to pass out a type that can be substituted
>>>>>>> - for both the RHS and the LHS types if there is a possibly
>>>>>>> - useless conversion involved. That means returning the
>>>>>>> - original RHS type as far as we can reconstruct it. */
>>>>>>> - if (code == GIMPLE_CALL)
>>>>>>> - {
>>>>>>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>>>> - if (gimple_call_internal_p (call_stmt)
>>>>>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>>>> - else
>>>>>>> - type = gimple_call_return_type (call_stmt);
>>>>>>> - }
>>>>>>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>>>> + if (gimple_call_internal_p (call_stmt)
>>>>>>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>>>> + type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>>>> + else
>>>>>>> + type = gimple_call_return_type (call_stmt);
>>>>>>> + return type;
>>>>>>
>>>>>> You might as well return the value directly and not use the variable.
>>>>>>
>>>>>>> + }
>>>>>>> + else if (code == GIMPLE_ASSIGN)
>>>>>>
>>>>>> else after return is kind of silly.
>>>>>>
>>>>>>> + {
>>>>>>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>>>>>> + type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>>>> else
>>>>>>> - switch (gimple_assign_rhs_code (stmt))
>>>>>>> - {
>>>>>>> - case POINTER_PLUS_EXPR:
>>>>>>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>>>> - break;
>>>>>>> -
>>>>>>> - default:
>>>>>>> - /* As fallback use the type of the LHS. */
>>>>>>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>>>> - break;
>>>>>>> - }
>>>>>>> + /* As fallback use the type of the LHS. */
>>>>>>> + type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>>>> return type;
>>>>>>
>>>>>> might as well not use type here either.
>>>>>>
>>>>>> Trev
>>>>>>
>>>>>>> }
>>>>>>> else if (code == GIMPLE_COND)
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>
>
>

RE: Refactor gimple_expr_type

[Aditya K]

[-- Attachment #1: Type: text/plain, Size: 9038 bytes --]



----------------------------------------
> Date: Wed, 20 May 2015 11:11:52 +0200
> Subject: Re: Refactor gimple_expr_type
> From: richard.guenther@gmail.com
> To: hiraditya@msn.com
> CC: tbsaunde@tbsaunde.org; gcc-patches@gcc.gnu.org
>
> On Tue, May 19, 2015 at 6:50 PM, Aditya K <hiraditya@msn.com> wrote:
>>
>>
>> ----------------------------------------
>>> Date: Tue, 19 May 2015 11:33:16 +0200
>>> Subject: Re: Refactor gimple_expr_type
>>> From: richard.guenther@gmail.com
>>> To: hiraditya@msn.com
>>> CC: tbsaunde@tbsaunde.org; gcc-patches@gcc.gnu.org
>>>
>>> On Tue, May 19, 2015 at 12:04 AM, Aditya K <hiraditya@msn.com> wrote:
>>>>
>>>>
>>>> ----------------------------------------
>>>>> Date: Mon, 18 May 2015 12:08:58 +0200
>>>>> Subject: Re: Refactor gimple_expr_type
>>>>> From: richard.guenther@gmail.com
>>>>> To: hiraditya@msn.com
>>>>> CC: tbsaunde@tbsaunde.org; gcc-patches@gcc.gnu.org
>>>>>
>>>>> On Sun, May 17, 2015 at 5:31 PM, Aditya K <hiraditya@msn.com> wrote:
>>>>>>
>>>>>>
>>>>>> ----------------------------------------
>>>>>>> Date: Sat, 16 May 2015 11:53:57 -0400
>>>>>>> From: tbsaunde@tbsaunde.org
>>>>>>> To: hiraditya@msn.com
>>>>>>> CC: gcc-patches@gcc.gnu.org
>>>>>>> Subject: Re: Refactor gimple_expr_type
>>>>>>>
>>>>>>> On Fri, May 15, 2015 at 07:13:35AM +0000, Aditya K wrote:
>>>>>>>> Hi,
>>>>>>>> I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if.
>>>>>>>>
>>>>>>>> Please review this patch.
>>>>>>>
>>>>>>> for some reason your mail client seems to be inserting non breaking
>>>>>>> spaces all over the place. Please either configure it to not do that,
>>>>>>> or use git send-email for patches.
>>>>>>
>>>>>> Please see the updated patch.
>>>>>
>>>>> Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type
>>>>> didn't exist btw...)
>>>>
>>>> Thanks for the review. Do you have any suggestions on how to remove gimple_expr_type. Are there any alternatives to it?
>>>> I can look into refactoring more (if it is not too complicated) since I'm already doing this.
>>>
>>> Look at each caller - usually they should be fine with using TREE_TYPE
>>> (gimple_get_lhs ()) (or a more specific one
>>> dependent on what stmts are expected at the place). You might want to
>>> first refactor the code
>>>
>>> else if (code == GIMPLE_COND)
>>> gcc_unreachable ();
>>>
>>> and deal with the fallout in callers (similar for the void_type_node return).
>>
>> Thanks for the suggestions. I looked at the use cases there are 47 usages in different files. That might be a lot of changes I assume, and would take some time.
>> This patch passes bootstrap and make check (although I'm not very confident that my way of make check ran all the regtests)
>>
>> If this patch is okay to merge please do that. I'll continue working on removing gimle_expr_type.
>
> Please re-send the patch as attachment, your mailer garbles the text
> (send mails as non-unicode text/plain).
>

Sure. I have attached the file.

Thanks,
-Aditya

> Richard.
>
>> Thanks,
>> -Aditya
>>
>>
>>>
>>> Richard.
>>>
>>>
>>>> -Aditya
>>>>
>>>>>
>>>>> Thanks,
>>>>> Richard.
>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>>>>>
>>>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>>>>
>>>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>>>> index 95e4fc8..3a83e8f 100644
>>>>>> --- a/gcc/gimple.h
>>>>>> +++ b/gcc/gimple.h
>>>>>> @@ -5717,36 +5717,26 @@ static inline tree
>>>>>> gimple_expr_type (const_gimple stmt)
>>>>>> {
>>>>>> enum gimple_code code = gimple_code (stmt);
>>>>>> -
>>>>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>>>>> + /* In general we want to pass out a type that can be substituted
>>>>>> + for both the RHS and the LHS types if there is a possibly
>>>>>> + useless conversion involved. That means returning the
>>>>>> + original RHS type as far as we can reconstruct it. */
>>>>>> + if (code == GIMPLE_CALL)
>>>>>> {
>>>>>> - tree type;
>>>>>> - /* In general we want to pass out a type that can be substituted
>>>>>> - for both the RHS and the LHS types if there is a possibly
>>>>>> - useless conversion involved. That means returning the
>>>>>> - original RHS type as far as we can reconstruct it. */
>>>>>> - if (code == GIMPLE_CALL)
>>>>>> - {
>>>>>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>>> - if (gimple_call_internal_p (call_stmt)
>>>>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>>> - else
>>>>>> - type = gimple_call_return_type (call_stmt);
>>>>>> - }
>>>>>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>>> + if (gimple_call_internal_p (call_stmt)
>>>>>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>>> + return TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>>> + else
>>>>>> + return gimple_call_return_type (call_stmt);
>>>>>> + }
>>>>>> + else if (code == GIMPLE_ASSIGN)
>>>>>> + {
>>>>>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>>>>> + return TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>>> else
>>>>>> - switch (gimple_assign_rhs_code (stmt))
>>>>>> - {
>>>>>> - case POINTER_PLUS_EXPR:
>>>>>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>>> - break;
>>>>>> -
>>>>>> - default:
>>>>>> - /* As fallback use the type of the LHS. */
>>>>>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>>> - break;
>>>>>> - }
>>>>>> - return type;
>>>>>> + /* As fallback use the type of the LHS. */
>>>>>> + return TREE_TYPE (gimple_get_lhs (stmt));
>>>>>> }
>>>>>> else if (code == GIMPLE_COND)
>>>>>> return boolean_type_node;
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> -Aditya
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> -Aditya
>>>>>>>>
>>>>>>>>
>>>>>>>> gcc/ChangeLog:
>>>>>>>>
>>>>>>>> 2015-05-15 hiraditya <hiraditya@msn.com>
>>>>>>>>
>>>>>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.
>>>>>>>>
>>>>>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>>>>>> index 95e4fc8..168d3ba 100644
>>>>>>>> --- a/gcc/gimple.h
>>>>>>>> +++ b/gcc/gimple.h
>>>>>>>> @@ -5717,35 +5717,28 @@ static inline tree
>>>>>>>> gimple_expr_type (const_gimple stmt)
>>>>>>>> {
>>>>>>>> enum gimple_code code = gimple_code (stmt);
>>>>>>>> -
>>>>>>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>>>>>>> + tree type;
>>>>>>>> + /* In general we want to pass out a type that can be substituted
>>>>>>>> + for both the RHS and the LHS types if there is a possibly
>>>>>>>> + useless conversion involved. That means returning the
>>>>>>>> + original RHS type as far as we can reconstruct it. */
>>>>>>>> + if (code == GIMPLE_CALL)
>>>>>>>> {
>>>>>>>> - tree type;
>>>>>>>> - /* In general we want to pass out a type that can be substituted
>>>>>>>> - for both the RHS and the LHS types if there is a possibly
>>>>>>>> - useless conversion involved. That means returning the
>>>>>>>> - original RHS type as far as we can reconstruct it. */
>>>>>>>> - if (code == GIMPLE_CALL)
>>>>>>>> - {
>>>>>>>> - const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>>>>> - if (gimple_call_internal_p (call_stmt)
>>>>>>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>>>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>>>>> - else
>>>>>>>> - type = gimple_call_return_type (call_stmt);
>>>>>>>> - }
>>>>>>>> + const gcall *call_stmt = as_a <const gcall *> (stmt);
>>>>>>>> + if (gimple_call_internal_p (call_stmt)
>>>>>>>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>>>>>> + type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>>>>>> + else
>>>>>>>> + type = gimple_call_return_type (call_stmt);
>>>>>>>> + return type;
>>>>>>>
>>>>>>> You might as well return the value directly and not use the variable.
>>>>>>>
>>>>>>>> + }
>>>>>>>> + else if (code == GIMPLE_ASSIGN)
>>>>>>>
>>>>>>> else after return is kind of silly.
>>>>>>>
>>>>>>>> + {
>>>>>>>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>>>>>>>> + type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>>>>> else
>>>>>>>> - switch (gimple_assign_rhs_code (stmt))
>>>>>>>> - {
>>>>>>>> - case POINTER_PLUS_EXPR:
>>>>>>>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>>>>>>>> - break;
>>>>>>>> -
>>>>>>>> - default:
>>>>>>>> - /* As fallback use the type of the LHS. */
>>>>>>>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>>>>> - break;
>>>>>>>> - }
>>>>>>>> + /* As fallback use the type of the LHS. */
>>>>>>>> + type = TREE_TYPE (gimple_get_lhs (stmt));
>>>>>>>> return type;
>>>>>>>
>>>>>>> might as well not use type here either.
>>>>>>>
>>>>>>> Trev
>>>>>>>
>>>>>>>> }
>>>>>>>> else if (code == GIMPLE_COND)
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>
>>
>>
 		 	   		  

[-- Attachment #2: gimple_expr_type.patch --]
[-- Type: application/octet-stream, Size: 2278 bytes --]

gcc/ChangeLog:

2015-05-15  hiraditya  <hiraditya@msn.com>

        * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if.

diff --git a/gcc/gimple.h b/gcc/gimple.h
index 95e4fc8..3a83e8f 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -5717,36 +5717,26 @@ static inline tree
 gimple_expr_type (const_gimple stmt)
 {
   enum gimple_code code = gimple_code (stmt);
-
-  if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
+  /* In general we want to pass out a type that can be substituted
+     for both the RHS and the LHS types if there is a possibly
+     useless conversion involved.  That means returning the
+     original RHS type as far as we can reconstruct it.  */
+  if (code == GIMPLE_CALL)
     {
-      tree type;
-      /* In general we want to pass out a type that can be substituted
-         for both the RHS and the LHS types if there is a possibly
-	 useless conversion involved.  That means returning the
-	 original RHS type as far as we can reconstruct it.  */
-      if (code == GIMPLE_CALL)
-	{
-	  const gcall *call_stmt = as_a <const gcall *> (stmt);
-	  if (gimple_call_internal_p (call_stmt)
-	      && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
-	    type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
-	  else
-	    type = gimple_call_return_type (call_stmt);
-	}
+      const gcall *call_stmt = as_a <const gcall *> (stmt);
+      if (gimple_call_internal_p (call_stmt)
+          && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
+        return TREE_TYPE (gimple_call_arg (call_stmt, 3));
+      else
+        return gimple_call_return_type (call_stmt);
+    }
+  else if (code == GIMPLE_ASSIGN)
+    {
+      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
+        return TREE_TYPE (gimple_assign_rhs1 (stmt));
       else
-	switch (gimple_assign_rhs_code (stmt))
-	  {
-	  case POINTER_PLUS_EXPR:
-	    type = TREE_TYPE (gimple_assign_rhs1 (stmt));
-	    break;
-
-	  default:
-	    /* As fallback use the type of the LHS.  */
-	    type = TREE_TYPE (gimple_get_lhs (stmt));
-	    break;
-	  }
-      return type;
+        /* As fallback use the type of the LHS.  */
+        return TREE_TYPE (gimple_get_lhs (stmt));
     }
   else if (code == GIMPLE_COND)
     return boolean_type_node;