[PATCH] D38528: Parallelize tail-merge string table construction.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

It is a bit unclear to me which test you want to change to show the effect of this patch.
I.e. I would expect either adding one new test or modifying one of the
existent, but you change the 3 tests here and it is a bit confusing IMO.

Also, since this patch really has an old base (you posted it in 2017),
do you think it is worth to update its description with fresh numbers?



================
Comment at: lld/ELF/SyntheticSections.cpp:2955
+// This works because, if strings S and T can be tail-merged, S and T
+// must have the same tail substring. Currently we use the last 4
+// characters for sharding.
----------------
nit: I might be mistaken, but I think here you need a comma: "Currently" -> "Currently, "


================
Comment at: lld/ELF/SyntheticSections.h:836
+
+  // Section size
+  size_t Size;
----------------
nit: no full stop at the end of the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D38528