Re: Refactor gimple_expr_type

[Richard Biener <richard.guenther@gmail.com>]

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