[PATCH] LowerBitSets: Use byte arrays instead of bit sets to represent in-memory bit sets.

[JF Bastien]

================
Comment at: include/llvm/Transforms/IPO/LowerBitSets.h:174
@@ +173,3 @@
+  /// The number of bytes allocated so far for each of the bits.
+  uint64_t BitAllocs[8];
+
----------------
Could you add a symbolic constant for 8? Also, why 8? Is it good for x86 but now as good for ARM?

================
Comment at: include/llvm/Transforms/IPO/LowerBitSets.h:178
@@ +177,3 @@
+    for (unsigned I = 0; I != 8; ++I)
+      BitAllocs[I] = 0;
+  }
----------------
`memset` or `std::fill`.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:34
@@ -33,3 +33,3 @@
 
-STATISTIC(NumBitSetsCreated, "Number of bitsets created");
+STATISTIC(NumByteArraysCreated, "Number of byte arrays created");
 STATISTIC(NumBitSetCallsLowered, "Number of bitset calls lowered");
----------------
kcc wrote:
> It would be interesting to see more details stats here, 
> e.g. how many bits were packed into how many bytes. 
+1

How many bits are unused (didn't fit) would be interesting.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:152
@@ +151,3 @@
+  unsigned Bit = 0;
+  for (unsigned I = 1; I != 8; ++I)
+    if (BitAllocs[I] < BitAllocs[Bit])
----------------
Update 8.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:298
@@ +297,3 @@
+  // Create globals to stand in for byte arrays and masks. These never actually
+  // get initialized, we rauw and erase them later in allocateByteArrays() once
+  // we know the offset and mask to use.
----------------
Capitalize RAUW.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:360
@@ -268,1 +359,3 @@
+                                      ByteArrayInfo *&BAI, Value *BitOffset) {
+  if (BSI.BitSize <= 64) {
     // If the bit set is sufficiently small, we can avoid a load by bit testing
----------------
Just to be sure I understand: this inlines small (<= 64 bit) bitsets. The current patch only benefits from co-locating larger bitsets, right? It would probably be good for the docs to specify this double approach to reducing bitset footprint.

http://reviews.llvm.org/D7954

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