RE: Refactor gimple_expr_type

[Aditya K <hiraditya at msn dot com>]

----------------------------------------
> 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)
>>
>>