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