[PATCH] Introduce bitset metadata format and bitset lowering pass.

[Peter Collingbourne]

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:44
@@ +43,3 @@
+
+}
+
----------------
kcc wrote:
> }   // namespace
Done.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:54
@@ +53,3 @@
+
+bool LowerBitSets::buildBitSets(Module &M, const DataLayout *DL) {
+  Function *BitSetTestFunc =
----------------
kcc wrote:
> I wonder if you could split this 300 LOC function into several smaller ones. 
Done.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:58
@@ +57,3 @@
+  if (!BitSetTestFunc)
+    return false;
+
----------------
jfb wrote:
> Shouldn't it be an error to try to run the bit set test pass without a module containing the `llvm.bitset.test` intrinsic?
This pass should work with any module, including modules that happen not to contain calls to the intrinsic.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:75
@@ +74,3 @@
+    if (!CI)
+      continue;
+
----------------
jfb wrote:
> What other valid uses of the intrinsic are there?
There shouldn't be any (or at least the verifier should have errored on them). I've changed this to a `cast`.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:89
@@ +88,3 @@
+    if (!Ins.second)
+      continue;
+
----------------
jfb wrote:
> I'm not sure I get what's going on here :-)
I've added a comment which should hopefully clarify things here.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:181
@@ +180,3 @@
+      std::vector<uint64_t> Offsets;
+      uint64_t Min = ~0UL, Max = 0;
+
----------------
jfb wrote:
> `UNINT64_MIN` or `std::numeric_limits<uint64_t>::min()` is nicer.
(I think you mean `::max`) Done.

http://reviews.llvm.org/D7288

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