From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6785 invoked by alias); 30 May 2018 22:22:38 -0000 Mailing-List: contact fortran-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: fortran-owner@gcc.gnu.org Received: (qmail 6675 invoked by uid 89); 30 May 2018 22:22:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=H*r:sk:a20-v6s, HTo:U*janus, demands X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-pf0-f174.google.com Received: from mail-pf0-f174.google.com (HELO mail-pf0-f174.google.com) (209.85.192.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 30 May 2018 22:22:31 +0000 Received: by mail-pf0-f174.google.com with SMTP id a20-v6so9719920pfo.0; Wed, 30 May 2018 15:22:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding; bh=/z24247jeqUulfZ8v0ygucvdUXCJ67YfANWKleyQXac=; b=oPXWChausICJYhgi47Cb4A/O3FO7qU0bntCRXqFc3C4YpIFod9iOwhs/hs996dSLJf bGcbS/UO7aNE0tLxgHkWtVsw2Z/a8zNl7q/kl7wXRgCc2PfQ9FeIob454UW3ZahByPHY Ht9xUAcprqmpmM1azPuFmQJQpqhu7rhXboTk6eTUapNK8qlNEP1H2UZU2FjXrXhYG73/ 3kL8JTNGe4VfHECfqOk4BGCK80y8c7zCFx34CBwdFNDhAQKrQ0YPlnt/L8xOwY8YKXEl hSZ53sFy+9ciW2X3Xdqo47Wy+iEakPg37ZjdecFn7Hk4fxU/gX4yE2ZDFQ8C5L0kc8AW 1bRQ== X-Gm-Message-State: ALKqPwcSYOMVwg0x74fo3sCSs+/+jeE7soR7t3DHfxMNzujZ3Yq9x7UG Gv5q7ukTM91M9ESVkUib237LDGyD X-Google-Smtp-Source: ADUXVKIAc8uRtkmrTH2BvAOGJ+wM44tKTXw87hZWPpxMSkHHKvVLcvd1EvhWxuzfU7rLS8NuPxe3kA== X-Received: by 2002:a63:b742:: with SMTP id w2-v6mr3517362pgt.343.1527718949773; Wed, 30 May 2018 15:22:29 -0700 (PDT) Received: from andrew-precision-3520.localnet (pool-239.obs.carnegiescience.edu. [192.91.178.239]) by smtp.gmail.com with ESMTPSA id e6-v6sm95957632pff.185.2018.05.30.15.22.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 30 May 2018 15:22:28 -0700 (PDT) From: Andrew Benson To: Janus Weil Cc: Richard Biener , "fortran@gcc.gnu.org" Subject: Re: Optimization of add_dt_to_dt_list() in resolve.c Date: Wed, 30 May 2018 22:22:00 -0000 Message-ID: <2073064.y6ql7gNeXd@andrew-precision-3520> User-Agent: KMail/5.2.3 (Linux/4.4.0-127-generic; KDE/5.36.0; x86_64; ; ) In-Reply-To: References: <6944935.KTWIneVxun@andrew-precision-3520> <2946507.05iJNWrpSJ@andrew-precision-3520> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg00137.txt.bz2 Hi Janus, On Wednesday, May 30, 2018 10:42:46 PM PDT Janus Weil wrote: > Hi Andrew, > > 2018-05-29 22:24 GMT+02:00 Andrew Benson : > > Yes - definitely possible to remove gfc_dt_list entirely - new patch is > > attached. > > since you already got some 'contentual' comments, I'll give you some > more 'formal' ones ... > > > + if (!derived->dt_next) { > + if (gfc_derived_types) { > + derived->dt_next = gfc_derived_types->dt_next; > + gfc_derived_types->dt_next = derived; > + } else { > + derived->dt_next = derived; > + } > + gfc_derived_types = derived; > + } > > Here and in some other hunks you're not conforming with the GNU coding > style, which demands that opening and closing braces should be on > separate lines (and at a new indentation level). Thanks - I'll fix those cases. > - for (dt = gfc_derived_types; dt; dt = dt->next) > - gfc_copy_dt_decls_ifequal (derived, dt->derived, false); > - > + if (gfc_derived_types) { > + dt = gfc_derived_types; > + for (;;) > + { > + gfc_copy_dt_decls_ifequal (derived, dt, false); > + if (dt->dt_next == gfc_derived_types) > + break; > + dt = dt->dt_next; > + } > + } > > Is there a particular reason why you changed the loop to "for (;;)" ? > I find the old style a bit clearer and more compact. Also I think it's > more common in gfortran. I changed the original for loop since it wasn't possible (as far as I could see) to have an exit condition that worked now that list is circularly linked (i.e. "dt" never becomes NULL, and testing for dt->dt_next == gfc_derived_types doesn't work as it would miss the final entry in the list). So, I used for(;;) and added the exit condition explicitly into the loop. But, I agree, it's not very clear. An alternative would be something such as: - for (dt = gfc_derived_types; dt; dt = dt->next) - gfc_copy_dt_decls_ifequal (derived, dt->derived, false); - + for (dt = gfc_derived_types; dt; dt = dt->dt_next) + { + gfc_copy_dt_decls_ifequal (derived, dt, false); + if (dt->dt_next == gfc_derived_types) + break; + } + which is more compact, and has the advantage that if doesn't require an "if (gfc_derived_types)". Do you think that's a better approach? > Btw, do you already have a copyright assignment on file? If not, > you'll probably need one (see https://gcc.gnu.org/contribute.html). I don't, so will need to get that figured out. > Thanks for your contribution and welcome to the gfortran team :) Thanks! -Andrew -- * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html * Galacticus: https://bitbucket.org/abensonca/galacticus