* [PATCH 0/2] Two minor partial symtab cleanups @ 2021-03-24 20:15 Tom Tromey 2021-03-24 20:15 ` [PATCH 1/2] Allow expand_symtabs_matching to examine imported psymtabs Tom Tromey 2021-03-24 20:15 ` [PATCH 2/2] Simplify psymbol_functions::expand_symtabs_matching Tom Tromey 0 siblings, 2 replies; 7+ messages in thread From: Tom Tromey @ 2021-03-24 20:15 UTC (permalink / raw) To: gdb-patches Here are two minor partial symtab cleanups, also coming from the larger quick symbol function cleanup series that I'm working on. Regression tested on x86-64 Fedora 32. Let me know what you think. Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] Allow expand_symtabs_matching to examine imported psymtabs 2021-03-24 20:15 [PATCH 0/2] Two minor partial symtab cleanups Tom Tromey @ 2021-03-24 20:15 ` Tom Tromey 2021-03-25 19:29 ` Simon Marchi 2021-03-24 20:15 ` [PATCH 2/2] Simplify psymbol_functions::expand_symtabs_matching Tom Tromey 1 sibling, 1 reply; 7+ messages in thread From: Tom Tromey @ 2021-03-24 20:15 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey Currently the psymtab variant of expand_symtabs_matching has this check: /* We skip shared psymtabs because file-matching doesn't apply to them; but we search them later in the loop. */ if (ps->user != NULL) continue; In a larger series I'm working on, it's convenient to remove this check. And, I noticed that a similar check is not done for expand_symtabs_with_fullname. So, it made sense to me to remove the check here as well. gdb/ChangeLog 2021-03-24 Tom Tromey <tom@tromey.com> * psymtab.c (psymbol_functions::expand_symtabs_matching): Remove "user" check. --- gdb/ChangeLog | 5 +++++ gdb/psymtab.c | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gdb/psymtab.c b/gdb/psymtab.c index 597817269c1..5a64166d983 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -1313,11 +1313,6 @@ psymbol_functions::expand_symtabs_matching if (ps->readin_p (objfile)) continue; - /* We skip shared psymtabs because file-matching doesn't apply - to them; but we search them later in the loop. */ - if (ps->user != NULL) - continue; - if (file_matcher) { bool match; -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Allow expand_symtabs_matching to examine imported psymtabs 2021-03-24 20:15 ` [PATCH 1/2] Allow expand_symtabs_matching to examine imported psymtabs Tom Tromey @ 2021-03-25 19:29 ` Simon Marchi 2021-03-26 18:17 ` Tom Tromey 0 siblings, 1 reply; 7+ messages in thread From: Simon Marchi @ 2021-03-25 19:29 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 2021-03-24 4:15 p.m., Tom Tromey wrote: > Currently the psymtab variant of expand_symtabs_matching has this > check: > > /* We skip shared psymtabs because file-matching doesn't apply > to them; but we search them later in the loop. */ > if (ps->user != NULL) > continue; > > In a larger series I'm working on, it's convenient to remove this > check. And, I noticed that a similar check is not done for > expand_symtabs_with_fullname. So, it made sense to me to remove the > check here as well. > > gdb/ChangeLog > 2021-03-24 Tom Tromey <tom@tromey.com> > > * psymtab.c (psymbol_functions::expand_symtabs_matching): Remove > "user" check. > --- > gdb/ChangeLog | 5 +++++ > gdb/psymtab.c | 5 ----- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/gdb/psymtab.c b/gdb/psymtab.c > index 597817269c1..5a64166d983 100644 > --- a/gdb/psymtab.c > +++ b/gdb/psymtab.c > @@ -1313,11 +1313,6 @@ psymbol_functions::expand_symtabs_matching > if (ps->readin_p (objfile)) > continue; > > - /* We skip shared psymtabs because file-matching doesn't apply > - to them; but we search them later in the loop. */ > - if (ps->user != NULL) > - continue; > - > if (file_matcher) > { > bool match; > I'm not against this change, but I don't understand it. It seems logical to me to skip shared psymtabs, because we'll reach them through some other psymtabs that include them. So, both versions seem correct, but maybe knowing why it's convenient to you would help. Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Allow expand_symtabs_matching to examine imported psymtabs 2021-03-25 19:29 ` Simon Marchi @ 2021-03-26 18:17 ` Tom Tromey 2021-03-26 18:27 ` Simon Marchi 0 siblings, 1 reply; 7+ messages in thread From: Tom Tromey @ 2021-03-26 18:17 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: >> - /* We skip shared psymtabs because file-matching doesn't apply >> - to them; but we search them later in the loop. */ >> - if (ps->user != NULL) >> - continue; >> - Simon> I'm not against this change, but I don't understand it. It seems Simon> logical to me to skip shared psymtabs, because we'll reach them through Simon> some other psymtabs that include them. So, both versions seem correct, Simon> but maybe knowing why it's convenient to you would help. If the shared psymtab has a file name, then that file name will not be checked by the file matcher. This doesn't matter directly now because file matching is often done via a different method, that does check these. However, with my proposed series to remove the redundant methods, this issue is exposed. Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Allow expand_symtabs_matching to examine imported psymtabs 2021-03-26 18:17 ` Tom Tromey @ 2021-03-26 18:27 ` Simon Marchi 0 siblings, 0 replies; 7+ messages in thread From: Simon Marchi @ 2021-03-26 18:27 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 2021-03-26 2:17 p.m., Tom Tromey wrote:>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: > >>> - /* We skip shared psymtabs because file-matching doesn't apply >>> - to them; but we search them later in the loop. */ >>> - if (ps->user != NULL) >>> - continue; >>> - > > Simon> I'm not against this change, but I don't understand it. It seems > Simon> logical to me to skip shared psymtabs, because we'll reach them through > Simon> some other psymtabs that include them. So, both versions seem correct, > Simon> but maybe knowing why it's convenient to you would help. > > If the shared psymtab has a file name, then that file name will not be > checked by the file matcher. This doesn't matter directly now because > file matching is often done via a different method, that does check > these. However, with my proposed series to remove the redundant > methods, this issue is exposed. Ok, thanks. Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] Simplify psymbol_functions::expand_symtabs_matching 2021-03-24 20:15 [PATCH 0/2] Two minor partial symtab cleanups Tom Tromey 2021-03-24 20:15 ` [PATCH 1/2] Allow expand_symtabs_matching to examine imported psymtabs Tom Tromey @ 2021-03-24 20:15 ` Tom Tromey 2021-03-25 19:31 ` Simon Marchi 1 sibling, 1 reply; 7+ messages in thread From: Tom Tromey @ 2021-03-24 20:15 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey I noticed that psymbol_functions::expand_symtabs_matching calls make_ignore_params once per psymtab that is matched. This seems possibly expensive, so this patch hoists the call out of the loop. gdb/ChangeLog 2021-03-24 Tom Tromey <tom@tromey.com> * psymtab.c (psymbol_functions::expand_symtabs_matching): Only call make_ignore_params once. --- gdb/ChangeLog | 5 +++++ gdb/psymtab.c | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/gdb/psymtab.c b/gdb/psymtab.c index 5a64166d983..1ea7376a8c8 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -1306,6 +1306,10 @@ psymbol_functions::expand_symtabs_matching for (partial_symtab *ps : require_partial_symbols (objfile)) ps->searched_flag = PST_NOT_SEARCHED; + gdb::optional<lookup_name_info> psym_lookup_name; + if (lookup_name != nullptr) + psym_lookup_name = lookup_name->make_ignore_params (); + for (partial_symtab *ps : m_partial_symtabs->range ()) { QUIT; @@ -1335,7 +1339,7 @@ psymbol_functions::expand_symtabs_matching if ((symbol_matcher == NULL && lookup_name == NULL) || recursively_search_psymtabs (ps, objfile, domain, - lookup_name->make_ignore_params (), + *psym_lookup_name, symbol_matcher)) { struct compunit_symtab *symtab = -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] Simplify psymbol_functions::expand_symtabs_matching 2021-03-24 20:15 ` [PATCH 2/2] Simplify psymbol_functions::expand_symtabs_matching Tom Tromey @ 2021-03-25 19:31 ` Simon Marchi 0 siblings, 0 replies; 7+ messages in thread From: Simon Marchi @ 2021-03-25 19:31 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 2021-03-24 4:15 p.m., Tom Tromey wrote: > I noticed that psymbol_functions::expand_symtabs_matching calls > make_ignore_params once per psymtab that is matched. This seems > possibly expensive, so this patch hoists the call out of the loop. > > gdb/ChangeLog > 2021-03-24 Tom Tromey <tom@tromey.com> > > * psymtab.c (psymbol_functions::expand_symtabs_matching): Only > call make_ignore_params once. > --- > gdb/ChangeLog | 5 +++++ > gdb/psymtab.c | 6 +++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/gdb/psymtab.c b/gdb/psymtab.c > index 5a64166d983..1ea7376a8c8 100644 > --- a/gdb/psymtab.c > +++ b/gdb/psymtab.c > @@ -1306,6 +1306,10 @@ psymbol_functions::expand_symtabs_matching > for (partial_symtab *ps : require_partial_symbols (objfile)) > ps->searched_flag = PST_NOT_SEARCHED; > > + gdb::optional<lookup_name_info> psym_lookup_name; > + if (lookup_name != nullptr) > + psym_lookup_name = lookup_name->make_ignore_params (); > + > for (partial_symtab *ps : m_partial_symtabs->range ()) > { > QUIT; > @@ -1335,7 +1339,7 @@ psymbol_functions::expand_symtabs_matching > > if ((symbol_matcher == NULL && lookup_name == NULL) > || recursively_search_psymtabs (ps, objfile, domain, > - lookup_name->make_ignore_params (), > + *psym_lookup_name, > symbol_matcher)) > { > struct compunit_symtab *symtab = > This LGTM, thanks. Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-03-26 18:27 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-24 20:15 [PATCH 0/2] Two minor partial symtab cleanups Tom Tromey 2021-03-24 20:15 ` [PATCH 1/2] Allow expand_symtabs_matching to examine imported psymtabs Tom Tromey 2021-03-25 19:29 ` Simon Marchi 2021-03-26 18:17 ` Tom Tromey 2021-03-26 18:27 ` Simon Marchi 2021-03-24 20:15 ` [PATCH 2/2] Simplify psymbol_functions::expand_symtabs_matching Tom Tromey 2021-03-25 19:31 ` Simon Marchi
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).