From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 63230 invoked by alias); 20 May 2015 09:11:57 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 63219 invoked by uid 89); 20 May 2015 09:11:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,FREEMAIL_REPLY,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-ob0-f175.google.com Received: from mail-ob0-f175.google.com (HELO mail-ob0-f175.google.com) (209.85.214.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 20 May 2015 09:11:54 +0000 Received: by obblk2 with SMTP id lk2so32119567obb.0 for ; Wed, 20 May 2015 02:11:52 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.182.56.4 with SMTP id w4mr22964636obp.79.1432113112491; Wed, 20 May 2015 02:11:52 -0700 (PDT) Received: by 10.76.115.167 with HTTP; Wed, 20 May 2015 02:11:52 -0700 (PDT) In-Reply-To: References: <20150516155311.GA15671@tsaunders-iceball.corp.tor1.mozilla.com> Date: Wed, 20 May 2015 09:25:00 -0000 Message-ID: Subject: Re: Refactor gimple_expr_type From: Richard Biener To: Aditya K Cc: Trevor Saunders , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg01792.txt.bz2 On Tue, May 19, 2015 at 6:50 PM, Aditya K 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 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 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 >>>>> >>>>> * 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 (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 (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 >>>>>>> >>>>>>> * 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 (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 (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) >>>>>>> >>>>>>> >>>>> >>>>> >>> > > >