[PATCH] D46672: COFF: ICF a section and its associated pdata section as a unit.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lld/COFF/ICF.cpp:133
+bool ICF::assocEquals(const SectionChunk *A, const SectionChunk *B) {
+  auto ChildClasses = [&](const SectionChunk *SC) {
+    std::vector<uint32_t> Classes;
----------------
ruiu wrote:
> Is there any way to write this without creating temporary std::vector objects?
We could use a pair of iterators instead, but then we would have to handle skipping the debug/CFG sections in the right places, and I'm not sure that it's worth it.


================
Comment at: lld/COFF/ICF.cpp:141
+  };
+  return ChildClasses(A) == ChildClasses(B);
+}
----------------
smeenai wrote:
> Is there a possibility of ChildClasses(A) and ChildClasses(B) having the same contents but in a different order? Would we want to ICF in that case? (Since right now the == is going to be doing an ordered comparison.)
It's theoretically possible, but it turns out that clang-cl (and probably cl as well?) always emits the associative sections in the same order. Initially I had a sort on the vector here, but I removed it because it turned out that it didn't make a difference when linking Chrome. MSVC doesn't seem to care about the order, so it would probably be fine to put it back.


https://reviews.llvm.org/D46672