[PATCH] D74971: Add a llvm::shuffle and use it in lld

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 21 10:32:59 PST 2020


MaskRay added inline comments.


================
Comment at: lld/test/ELF/shuffle-sections.s:9
 
+## --shuffle-sections= shuffles input sections.
+# RUN: ld.lld --shuffle-sections=1 %t.o -o %t1.out
----------------
Also delete mention of `std::shuffle` in `shuffle-sections-init-fini.s`

I think keeping `ORDERED-SAME` and `SHUFFLED-NOT` in `shuffle-sections-init-fini.s` as is is fine, though I will not object if you also change them to test a particular order.


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:1014
+void shuffle(Iterator first, Iterator last, RNG &&g) {
+  --last;
+  for (; first < last; ++first) {
----------------
Return if `first == last`


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:1015
+  --last;
+  for (; first < last; ++first) {
+    // It would be better to use a std::uniform_int_distribution,
----------------
`<` -> `!=`


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:1018
+    // but that would be stdlib dependent.
+    auto d = last - first + 1;
+    auto i = g() % d;
----------------
d can be inlined.


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

https://reviews.llvm.org/D74971





More information about the llvm-commits mailing list