[PATCH] LowerBitSets: Introduce global layout builder.

[JF Bastien]

================
Comment at: include/llvm/Transforms/IPO/LowerBitSets.h:120
@@ +119,3 @@
+  /// The computed layout. Clients should use a nested pair of for loops to
+  /// access the object indices.
+  std::vector<std::vector<uint64_t>> Fragments;
----------------
I'm not sure I understand how "Clients should use a nested pair of for loops to access the object indices." is helpful. Are you trying to illustrate the order of indices?

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:534
@@ +533,3 @@
+    if (BitSetNM) {
+      for (MDNode *Op : BitSetNM->operands()) {
+        if (!Op->getOperand(1))
----------------
Could you clarify below what operands 0 and 1 are? It's not obvious from the surrounding code.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:552
@@ +551,3 @@
+        [](const std::set<uint64_t> &O1, const std::set<uint64_t> &O2) {
+          return O1.size() < O2.size();
+        });
----------------
pcc wrote:
> pcc wrote:
> > jfb wrote:
> > > pcc wrote:
> > > > jfb wrote:
> > > > > pcc wrote:
> > > > > > jfb wrote:
> > > > > > > This won't be deterministic without a tie-breaker?
> > > > > > Because I'm using `stable_sort`, it should be as deterministic as `EquivalenceClasses` is. I'm not certain, but it looks like the only non-determinism there relates to the order in which elements are visited (i.e. begin()..end()), not the order in which members of equivalence classes are visited (member_begin()..member_end()). That's a separate problem which we should fix somehow.
> > > > > My main concern is that order of iteration of `std::set` isn't stable, am I understanding correctly?
> > > > I don't understand. We aren't iterating over `std::set` here. `BitSetMembers` is a `std::vector`.
> > > Sorry, explaining myself badly: if the `std::set` are equal then we can't iterate through them to compare members? I guess your point was that using `std::stable_sort` keeps the same order as `BitSetMembers` already had, and that was already deterministic?
> > I still don't get it. We aren't iterating over any `std::set`s here, we are only taking their sizes. Anyway, `std::set`s are ordered, see http://en.cppreference.com/w/cpp/container/set
> Oh, and `BitSetMembers` is deterministic within the equivalence class (I think).
Oh I get it now, I was indeed confused :-)

http://reviews.llvm.org/D7796

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/