On Tue, May 19, 2015 at 6:50 PM, Aditya K <hiradi...@msn.com> wrote:
>
>
> ----------------------------------------
>> Date: Tue, 19 May 2015 11:33:16 +0200
>> Subject: Re: Refactor gimple_expr_type
>> From: richard.guent...@gmail.com
>> To: hiradi...@msn.com
>> CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org
>>
>> On Tue, May 19, 2015 at 12:04 AM, Aditya K <hiradi...@msn.com> wrote:
>>>
>>>
>>> ----------------------------------------
>>>> Date: Mon, 18 May 2015 12:08:58 +0200
>>>> Subject: Re: Refactor gimple_expr_type
>>>> From: richard.guent...@gmail.com
>>>> To: hiradi...@msn.com
>>>> CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org
>>>>
>>>> On Sun, May 17, 2015 at 5:31 PM, Aditya K <hiradi...@msn.com> wrote:
>>>>>
>>>>>
>>>>> ----------------------------------------
>>>>>> Date: Sat, 16 May 2015 11:53:57 -0400
>>>>>> From: tbsau...@tbsaunde.org
>>>>>> To: hiradi...@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 <hiradi...@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 <hiradi...@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)
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>
>
>
