On Thu, Aug 8, 2019 at 2:24 PM Michael Matz <m...@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.