This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Restrict LOOP_ALIGN to loop headers only.
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Michael Matz <matz at suse dot de>
- Cc: Martin Liška <mliska at suse dot cz>, Jan Hubicka <hubicka at ucw dot cz>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 9 Aug 2019 13:15:08 +0200
- Subject: Re: [PATCH] Restrict LOOP_ALIGN to loop headers only.
- References: <90fcdb35-bd56-1614-041d-58d274d46074@suse.cz> <20190709095617.7aflaclq6jel37jz@kam.mff.cuni.cz> <CAFiYyc107aYymnz-LXiFuwtFukRwvgaxYV+CFiwUejj54-=P6w@mail.gmail.com> <CAFiYyc2aJXZFzCeZ1p71v3011puNZSS9Uj+Z1_JamoVjNLjtTg@mail.gmail.com> <CAFiYyc0mjbcW338bj8e3BihyuaQGzsj2-mFhbW5JBxRKaMdBZw@mail.gmail.com> <alpine.LSU.2.21.1907091306530.22824@wotan.suse.de> <A945EEEC-7085-4D90-AB51-297BCEDB4DD8@gmail.com> <alpine.LSU.2.21.1907101157080.22824@wotan.suse.de> <05AFA22F-7EE2-442F-8F64-558CD743303F@gmail.com> <CAFiYyc1=ZVJOO-fj8WBaV0kr6XhnZ=E3yD4iTZ57YJsdYOkHxA@mail.gmail.com> <4bf00f1e-a129-7e38-b6cb-4ce7c8bfc96b@suse.cz> <alpine.LSU.2.21.1908081220340.25869@wotan.suse.de>
On Thu, Aug 8, 2019 at 2:24 PM Michael Matz <matz@suse.de> wrote:
>
> Hi,
>
> On Thu, 8 Aug 2019, Martin Liška wrote:
>
> > > So docs have
> > >
> > > @defmac JUMP_ALIGN (@var{label})
> > > The alignment (log base 2) to put in front of @var{label}, which is
> > > a common destination of jumps and has no fallthru incoming edge.
>
> So, per docu: JUMP_ALIGN implies !fallthru ...
>
> > > align_flags alignment = has_fallthru ? JUMP_ALIGN (label) :
> > > LOOP_ALIGN (label);
>
> ... exactly the opposite way here.
Yeah, sorry - my mistake.
> > > instead of the two different conditions? Or the JUMP_ALIGN case
> > > computing "common destination" instead of "frequently executed"
> > > somehow but I think it makes sense that those are actually the same
> > > here (frequently executed). It oddly looks at prev_bb and is not
> > > guarded with optimize_bb_for_speed_p at all so this all is
> > > full of heuristics and changing anything here just based on x86
> > > measurements is surely going to cause issues for targets more
> > > sensitive to (mis-)alignment.
> >
> > I like you patch, it's a rapid simplification of the code which
> > we have there.
>
> Yeah, but it's also contradicting the documentation. And I think the docu
> makes sense, because it means that there is no padding inserted on the
> fall-thru path (because there is none). So please measure with the
> opposite direction. (I still think these conditions shouldn't be
> hot-needled, but rather should somewhat make sense in the abstract).
Of course I'm still afraid that the other code exists for a reason
(tuning/hack/whatever...).
Note that with the patch we're now applying LOOP_ALIGN to L2 here:
if (a)
foo = bar;
L2:
blah;
because there's a jump-around and a fallthru. So I'm not sure
we don't need to apply some condition on fallthru_count
(which is unused after your patch btw).
Richard.
>
> Ciao,
> Michael.