[PATCH] D57666: [LLD] [COFF] Avoid O(n^2) insertion into PartialSections

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 4 13:08:18 PST 2019


mstorsjo marked an inline comment as done.
mstorsjo added a comment.

In D57666#1383871 <https://reviews.llvm.org/D57666#1383871>, @aganea wrote:

> In D57666#1383852 <https://reviews.llvm.org/D57666#1383852>, @mstorsjo wrote:
>
> > Hmm, I don't really see where PartialSection or the set/map PartialSections are used within that patch at all?
>
>
> Sorry about the confusion. It was callled `InputSection` in D54802 <https://reviews.llvm.org/D54802>, see lld/trunk/COFF/Writer.h in that patch.
>
> > But even then, if it isn't part of the PartialSection object itself, but only part of the key in the map, can't you get it while iterating over it, like this?
>
> Not for iterating, but for referencing a `PartialSection` from an `OutputSection` for later usage, see lld/trunk/COFF/PDB.cpp, L1509.


Oh, I see. Yes, then the approach of this patch, duplicating it in the key and within the value, seems reasonable.



================
Comment at: COFF/Writer.cpp:1787
+  PartialSectionKey Key = {Name, OutChars};
+  return PartialSections[Key];
 }
----------------
rnk wrote:
> aganea wrote:
> > `return PartialSections[{Name, OutChars}];` ?
> When looking for keys not in the map, this has the side effect of creating a new (null) map entry. I think the usual .find() / != .end() dance is probably what we want instead.
Oh, right, will fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57666/new/

https://reviews.llvm.org/D57666





More information about the llvm-commits mailing list