---------------------------------------- > Date: Wed, 20 May 2015 11:11:52 +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 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). >
Sure. I have attached the file.
Thanks,
-Aditya
> 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)
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>
>>
>>
gimple_expr_type.patch
Description: Binary data
