* Refactor gimple_expr_type @ 2015-05-15 7:57 Aditya K 2015-05-16 16:36 ` Trevor Saunders 0 siblings, 1 reply; 9+ messages in thread From: Aditya K @ 2015-05-15 7:57 UTC (permalink / raw) To: gcc-patches 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. 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; + } + else if (code == GIMPLE_ASSIGN) + { + 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; } else if (code == GIMPLE_COND) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Refactor gimple_expr_type 2015-05-15 7:57 Refactor gimple_expr_type Aditya K @ 2015-05-16 16:36 ` Trevor Saunders 2015-05-17 17:48 ` Aditya K 0 siblings, 1 reply; 9+ messages in thread From: Trevor Saunders @ 2015-05-16 16:36 UTC (permalink / raw) To: Aditya K; +Cc: gcc-patches 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. > > 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) > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: Refactor gimple_expr_type 2015-05-16 16:36 ` Trevor Saunders @ 2015-05-17 17:48 ` Aditya K 2015-05-18 10:26 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Aditya K @ 2015-05-17 17:48 UTC (permalink / raw) To: Trevor Saunders; +Cc: gcc-patches ---------------------------------------- > 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. 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) >> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Refactor gimple_expr_type 2015-05-17 17:48 ` Aditya K @ 2015-05-18 10:26 ` Richard Biener 2015-05-18 22:24 ` Aditya K 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2015-05-18 10:26 UTC (permalink / raw) To: Aditya K; +Cc: Trevor Saunders, gcc-patches 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, 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) >>> >>> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: Refactor gimple_expr_type 2015-05-18 10:26 ` Richard Biener @ 2015-05-18 22:24 ` Aditya K 2015-05-19 9:39 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Aditya K @ 2015-05-18 22:24 UTC (permalink / raw) To: Richard Biener; +Cc: Trevor Saunders, gcc-patches ---------------------------------------- > 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) >>>> >>>> >> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Refactor gimple_expr_type 2015-05-18 22:24 ` Aditya K @ 2015-05-19 9:39 ` Richard Biener 2015-05-19 16:50 ` Aditya K 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2015-05-19 9:39 UTC (permalink / raw) To: Aditya K; +Cc: Trevor Saunders, gcc-patches On Tue, May 19, 2015 at 12:04 AM, Aditya K <hiraditya@msn.com> wrote: > > > ---------------------------------------- >> 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. 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 <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) >>>>> >>>>> >>> >>> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: Refactor gimple_expr_type 2015-05-19 9:39 ` Richard Biener @ 2015-05-19 16:50 ` Aditya K 2015-05-20 9:25 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Aditya K @ 2015-05-19 16:50 UTC (permalink / raw) To: Richard Biener; +Cc: Trevor Saunders, gcc-patches ---------------------------------------- > Date: Tue, 19 May 2015 11:33:16 +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 Tue, May 19, 2015 at 12:04 AM, Aditya K <hiraditya@msn.com> wrote: >> >> >> ---------------------------------------- >>> 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. > > 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. Thanks, -Aditya > > Richard. > > >> -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) >>>>>> >>>>>> >>>> >>>> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Refactor gimple_expr_type 2015-05-19 16:50 ` Aditya K @ 2015-05-20 9:25 ` Richard Biener 2015-05-20 13:33 ` Aditya K 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2015-05-20 9:25 UTC (permalink / raw) To: Aditya K; +Cc: Trevor Saunders, gcc-patches On Tue, May 19, 2015 at 6:50 PM, Aditya K <hiraditya@msn.com> wrote: > > > ---------------------------------------- >> Date: Tue, 19 May 2015 11:33:16 +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 Tue, May 19, 2015 at 12:04 AM, Aditya K <hiraditya@msn.com> wrote: >>> >>> >>> ---------------------------------------- >>>> 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. >> >> 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 <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) >>>>>>> >>>>>>> >>>>> >>>>> >>> > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: Refactor gimple_expr_type 2015-05-20 9:25 ` Richard Biener @ 2015-05-20 13:33 ` Aditya K 0 siblings, 0 replies; 9+ messages in thread From: Aditya K @ 2015-05-20 13:33 UTC (permalink / raw) To: Richard Biener; +Cc: Trevor Saunders, gcc-patches [-- Attachment #1: Type: text/plain, Size: 9038 bytes --] ---------------------------------------- > Date: Wed, 20 May 2015 11:11:52 +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 Tue, May 19, 2015 at 6:50 PM, Aditya K <hiraditya@msn.com> wrote: >> >> >> ---------------------------------------- >>> Date: Tue, 19 May 2015 11:33:16 +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 Tue, May 19, 2015 at 12:04 AM, Aditya K <hiraditya@msn.com> wrote: >>>> >>>> >>>> ---------------------------------------- >>>>> 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. >>> >>> 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 <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) >>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>> >> >> >> [-- Attachment #2: gimple_expr_type.patch --] [-- Type: application/octet-stream, Size: 2278 bytes --] 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; ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-05-20 13:29 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-15 7:57 Refactor gimple_expr_type Aditya K 2015-05-16 16:36 ` Trevor Saunders 2015-05-17 17:48 ` Aditya K 2015-05-18 10:26 ` Richard Biener 2015-05-18 22:24 ` Aditya K 2015-05-19 9:39 ` Richard Biener 2015-05-19 16:50 ` Aditya K 2015-05-20 9:25 ` Richard Biener 2015-05-20 13:33 ` Aditya K
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).