[PATCH] D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections.

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi marked 5 inline comments as done.
plotfi added inline comments.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:426
+
+    for (auto &Sec : Obj.sections()) {
+      RelocationSection *R = dyn_cast<RelocationSection>(&Sec);
----------------
jhenderson wrote:
> plotfi wrote:
> > jhenderson wrote:
> > > This is a bad way to do this. It means you'll iterate over the whole set of sections once for each compressed section again, just looking for a specific relocation section that may not even exist.
> > > 
> > > I think the better way to do it would be to make your vector a map, mapping relocation sections to their debug section, and then it can be done in the same loop as above, where you are already iterating over all sections. Something like this partial pseudo-code:
> > > 
> > > ```
> > > Map<SectionBase *, SmallVector<SectionBase*, 1>> ToCompress;
> > > for (auto &Sec : Obj.sections()) {
> > >   if (shouldCompress(Sec))
> > >     ToCompress.insert(Sec);
> > >   else if (auto RelocSec = dyn_cast<RelocationSection>(Sec)) {
> > >     auto TargetSection = RelocSec->getSection();
> > >     if (shouldCompress(TargetSection))
> > >       ToCompress[TargetSection].push_back(RelocSection);
> > >   }
> > > }
> > > ```
> > > Two things to watch out for: 1) it is technically legal to have multiple relocation sections targeting the same section, so it needs to be a vector not a single section. 2) It is possible for the relocation section to be before or after the target section in the section list.
> > For now I've added a more simple solution. In the top-level loop I collect the relocation sections that point to compresable debug sections and then in the next loop's inner loop I check for the relocation.
> I suppose this works, and it shouldn't cause a performance issue overall, but it seems clunky to do it this way, since you already have this information calculated in the first loop. You're just recalculating it again effectively.
Yes, I know. Just keeping things simple. 


Repository:
  rL LLVM

https://reviews.llvm.org/D49678