Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Simon Marchi <simark@simark.ca>]

On 2020-02-23 6:58 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> I think it would be more natural and easier to understand to have :
> 
> Simon>   objfile -> dwarf2 objfile-specific object -> dwarf2 object shared between objfiles
> 
> Simon> Many "dwarf2 objfile-specific" objects would point to one "dwarf2 object shared between
> Simon> objfiles", without special tricks.  Of course, it's much easier said than done.
> 
> Simon> I'd like to know: did you consider this way of doing it and chose not to do it
> Simon> because of the complexity / size of the change?  Is the current patchset a stepping
> Simon> stone towards it?
> 
> I was afraid of the size of the resulting patch.  However, I think
> you're right that this would be better.  And, I feel pretty strongly
> that we should be making the DWARF reader less awful, not more.

Yeah, as I already showed you, I started making such a patch (more details
below).  I think it's worth doing this work, because in the end it's easier
to follow the code.  I find that the dwarf2_enter_objfile approach is
analogous to having to set a "current context" (e.g. call switch_to_thread)
before calling a certain function, which goes against what we are trying
to do in general (for example, trying to change functions to accept a
thread / ptid rather than rely on inferior_ptid).

Since dwarf2_per_cu_data objects become objfile-independent, functions that
need to work on a dwarf2_per_cu_data in an objfile-dependent way then need
to accept an objfile or dwarf2_per_objfile object along with the
dwarf2_per_cu_data.  It's a bit tedious, but in the end it forces us to pass
the objfile context all the way from the callers, and it's less likely that
we forget a spot, like we could with dwarf2_enter_objfile, since it would just
not compile.

> Probably the thing to do is reorder the series a little (some patches
> don't depend on this), then redo the "unsharing" patch, and finally
> update the subsequent patches.
> 
> A few of these patches (1-3, 5, and 11) could probably go in sooner.
> What do you think of that?

#1 is already in, right?
#2: Yes, sounds like a good cleanup.
#4: Yes, good cleanup.

#3: It's only useful if we end up doing psymtabs sharing, so I would not push
    it yet.
#5 and #11: I'm not sure how much of that I end up un-doing in my prototype patch,
    swapping the shareable and unshareable parts.  And it is also only useful in
    the context of psymtab sharing, so I wouldn't push it yet.

I'm currently experimenting with what I suggested above, this is what I have for
now (this will get force-pushed as I progress):

  https://github.com/simark/binutils-gdb/commits/share-psymtabs

The status is: I'm based on your patch 9/14 ("Add objfile member to DWARF batons"),
as it aims mainly to replace patch patch 10/14 ("Introduce dwarf2_enter_objfile and
use it") with the architecture I proposed.  I see no regression in a simple
"make check" between it and the previous commit.  I have yet to apply and adapt the
rest of your series on top of my commit.  I would also like to test with the variety
of testsuite boards that Tom de Vries added, since they will test various DWARF
configurations (and I would feel bad if he had to clean up after us again).

Simon