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