From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12605 invoked by alias); 25 Oct 2018 15:01:51 -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 12593 invoked by uid 89); 25 Oct 2018 15:01:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=BAYES_00,GIT_PATCH_1,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=versioning, sk:pass_lo, beneficiaries X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 25 Oct 2018 15:01:45 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 59DBA3002B06; Thu, 25 Oct 2018 15:01:44 +0000 (UTC) Received: from ovpn-116-164.phx2.redhat.com (ovpn-116-164.phx2.redhat.com [10.3.116.164]) by smtp.corp.redhat.com (Postfix) with ESMTP id C277527BB1; Thu, 25 Oct 2018 15:01:43 +0000 (UTC) Message-ID: <1540479702.14521.304.camel@redhat.com> Subject: Re: Add a loop versioning pass From: David Malcolm To: Richard Sandiford , gcc-patches@gcc.gnu.org Date: Thu, 25 Oct 2018 16:16:00 -0000 In-Reply-To: <874ldb1omb.fsf@arm.com> References: <874ldb1omb.fsf@arm.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-10/txt/msg01602.txt.bz2 On Wed, 2018-10-24 at 14:05 +0100, Richard Sandiford wrote: > This patch adds a pass that versions loops with variable index > strides > for the case in which the stride is 1. E.g.: > > for (int i = 0; i < n; ++i) > x[i * stride] = ...; > > becomes: > > if (stepx == 1) > for (int i = 0; i < n; ++i) > x[i] = ...; > else > for (int i = 0; i < n; ++i) > x[i * stride] = ...; > > This is useful for both vector code and scalar code, and in some > cases > can enable further optimisations like loop interchange or pattern > recognition. > > The pass gives a 7.6% improvement on Cortex-A72 for 554.roms_r at -O3 > and a 2.4% improvement for 465.tonto. I haven't found any SPEC tests > that regress. > > Sizewise, there's a 10% increase in .text for both 554.roms_r and > 465.tonto. That's obviously a lot, but in tonto's case it's because > the whole program is written using assumed-shape arrays and pointers, > so a large number of functions really do benefit from versioning. > roms likewise makes heavy use of assumed-shape arrays, and that > improvement in performance IMO justifies the code growth. > > The next biggest .text increase is 4.5% for 548.exchange2_r. I did > see > a small (0.4%) speed improvement there, but although both 3-iteration > runs > produced stable results, that might still be noise. There was a > slightly > larger (non-noise) improvement for a 256-bit SVE model. > > 481.wrf and 521.wrf_r .text grew by 2.8% and 2.5% respectively, but > without any noticeable improvement in performance. No other test > grew > by more than 2%. > > Although the main SPEC beneficiaries are all Fortran tests, the > benchmarks we use for SVE also include some C and C++ tests that > benefit. > > Using -frepack-arrays gives the same benefits in many Fortran cases. > The problem is that using that option inappropriately can force a > full > array copy for arguments that the function only reads once, and so it > isn't really something we can turn on by default. The new pass is > supposed to give most of the benefits of -frepack-arrays without > the risk of unnecessary repacking. > > The patch therefore enables the pass by default at -O3. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > Richard > [...snip...] > +/* Run the pass and return a set of TODO_* flags. */ > + > +unsigned int > +loop_versioning::run () > +{ > + gcc_assert (scev_initialized_p ()); > + > + if (!analyze_blocks () > + || !prune_conditions () > + || !make_versioning_decisions () > + || !implement_versioning_decisions ()) > + return 0; > + > + return TODO_update_ssa; > +} > + > +/* Loop versioningting pass. */ (typo) > + > +namespace { Could the whole file be within this anonymous namespace, rather than just the opt_pass subclass? (hiding class loop_versioning, so that the optimizer knows that the only thing visible outside the TU is make_pass_loop_versioning). This can be a pain to debug, but you can always comment out the anon namespace locally when debugging. > + > +const pass_data pass_data_loop_versioning = > +{ > + GIMPLE_PASS, /* type */ > + "lversion", /* name */ > + OPTGROUP_LOOP, /* optinfo_flags */ > + TV_LOOP_VERSIONING, /* tv_id */ > + PROP_cfg, /* properties_required */ > + 0, /* properties_provided */ > + 0, /* properties_destroyed */ > + 0, /* todo_flags_start */ > + 0, /* todo_flags_finish */ > +}; [...snip...] > + > +} // anon namespace > + > +gimple_opt_pass * > +make_pass_loop_versioning (gcc::context *ctxt) > +{ > + return new pass_loop_versioning (ctxt); > +} [...snip...]