This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: Refactor gimple_expr_type



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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]