[PATCH] D11856: LowerBitSets: Extend pass to support functions as bitset members.

[Kostya Serebryany via llvm-commits]

kcc added inline comments.

================
Comment at: docs/BitSets.rst:13
@@ -12,4 +12,3 @@
 ``llvm.bitsets``.  Each element is a metadata node with three elements:
-the first is a metadata string containing an identifier for the bitset,
-the second is a global variable and the third is a byte offset into the
-global variable.
+the first is a metadata object representing an identifier for the bitset,
+the second is either a global variable or a function and the third is a
----------------
maybe a bullet/numbered list? 

================
Comment at: docs/BitSets.rst:31
@@ +30,3 @@
+is no guarantee that its identity within the module will be the same as its
+identity outside of the module, as the former will be the jump table entry
+if a jump table is necessary. However, the identity of functions defined
----------------
You introduce the concept of jump tables w/o explaining them. 
Maybe add a paragraph telling what jump table is? 

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:749
@@ -604,1 +748,3 @@
 
+  // Verify the bitset metadata and build a mapping from bitset identifiers to
+  // their last observed index in BitSetNM. This will used later to
----------------
quite a bit of code in a single function. Maybe split it into several? 

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:902
@@ +901,3 @@
+    if (Globals.empty() || isa<GlobalVariable>(Globals[0])) {
+      // Build a vector of global variables with the computed layout.
+      std::vector<GlobalVariable *> OrderedGVs(Globals.size());
----------------
Two pieces of very similar code. 
Can we use some C++ magic to not duplicate the code (template? use the parent class?)


http://reviews.llvm.org/D11856