public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: Alexandre Oliva <aoliva@redhat.com>
Cc: binutils@sourceware.org
Subject: Re: [PR24444] speed up locview resolution wiht relaxable frags
Date: Wed, 17 Apr 2019 01:32:00 -0000	[thread overview]
Message-ID: <20190417013218.GA14424@bubble.grove.modra.org> (raw)
In-Reply-To: <orftqiw0az.fsf@lxoliva.fsfla.org>

On Tue, Apr 16, 2019 at 10:24:04AM -0300, Alexandre Oliva wrote:
> On Apr 14, 2019, Alan Modra <amodra@gmail.com> wrote:
> 
> > Yes, I believe so.  In fact, fr_fix is really unsigned.  Also, it is
> > an error for rs_org frags to go backwards.
> 
> Nice, that makes things simpler.
> 
> >> Tested on x86_64-linux-gnu, native and cross to xtensa-elf.  No new
> >> testcase; the issue was just performance, not output correctness.  Ok to
> >> install?
> 
> > Um, the testcase object file after this patch differs from the
> > original.  First readelf -wi difference shown below.
> 
> Which testcase was that?  I didn't catch that one.

The https://sourceware.org/bugzilla/show_bug.cgi?id=24444 reproducer
attachment.

I see differences in DW_AT_GNU_entry_view, in all cases substituting
3 for 10, or 2 for 9 (lower number after your patch).  I don't know
enough to say whether this is a reasonable result.

> 
> >> +  /* If any frag from frag2, inclusive, to frag1, exclusive, has a
> >> +     nonzero fixed length, the addresses won't be the same as long as
> >> +     we reach frag1.  */
> >> +  bfd_boolean maybe_same_addr = off1 == 0 && off2 == (valueT)frag2->fr_fix;
> 
> > Ins't it true that if the symbol offsets are not at exactly the above
> > values, then you already know the result and there's no need to
> > traverse the frags?  This is assuming you can't .org backwards, which
> > is the case.
> 
> Not entirely, no.  Although the comments state we assume a certain
> ordering of frags, the assumption is only valid for O_gt operations
> arising from the location view machinery.  It doesn't hurt if we resolve
> other O_gt operations if we can find the result, but those could have
> frags in the opposite order.

Thanks for reminding me.  I wrote that comment before writing the
simplification of your patch, and there you'll note I did chase down
the second frag..

> Now, O_gt is not exactly the operation the location view machinery
> needs; O_ne (or, after negation, O_eq) would be more like it.  IIRC the
> reason I went for O_gt was just that it was resolved early in a lot more
> cases than O_eq.  Now, with this kind of fallback, we could actually
> introduce another operation kind that (i) resolves to zero if frags are
> not in the same (sub)section, and (ii) can safely make the optimization
> you suggested for frags in the same (sub)section, with the extra benefit
> that we know we won't ever search all the way to the end of the frag
> linked list without finding the other frag: we could assert-check that,
> and stop the search at the first nonzero-fr_fix.
> 
> Would a new op with this semantics (O_incview?, vs reset view) be
> preferred?

No, if O_gt works let's go with that.

> > The following makes the changes I'm suggesting, and simplifies a few
> > more things.
> 
> Thanks, I'll wait for a response to the question above before taking
> further action on this, but I'll likely integrate your change (credited
> in the ChangeLog) in the patch regardless.

-- 
Alan Modra
Australia Development Lab, IBM

  reply	other threads:[~2019-04-17  1:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-13  8:57 Alexandre Oliva
2019-04-14 13:22 ` Alan Modra
2019-04-16 13:24   ` Alexandre Oliva
2019-04-17  1:32     ` Alan Modra [this message]
2019-04-24 23:18     ` Alan Modra
2019-04-30  5:49       ` Alexandre Oliva
2019-05-03  1:14         ` Alan Modra
2019-05-04  0:56           ` Alexandre Oliva
2019-05-04  3:22         ` Alexandre Oliva
2019-05-04  5:44           ` Alan Modra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190417013218.GA14424@bubble.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=aoliva@redhat.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).