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

[Kostya Serebryany]

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:168
@@ +167,3 @@
+LowerBitSets::BitSetInfo LowerBitSets::buildBitSet(
+    Module &M,
+    MDString *BitSet,
----------------
do you need M here? 

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:197
@@ +196,3 @@
+  }
+
+  if (Min > Max)
----------------
I'd split this function into two at this point. 
Then the second part of the function will become easily unit-testable. 

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:201
@@ +200,3 @@
+
+  // Normalize each offset against the minimum observed offset, and compute
+  // the bitwise OR of each of the offsets. The number of trailing zeros
----------------
This part is the key to understanding what we are doing here. 
It would be nice to have examples of input (array of offsets) and output (bitset). 
But then, a unit tests would be a better example than comments.
I do not agree with you here that we have enough test coverage and hence do not need more tests. 
unit tests are not necessarily required only for test coverage -- they are also a good way to provide examples.  

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:293
@@ +292,3 @@
+  // instruction with the previously computed bit offset; LLVM does not
+  // currently expose this instruction.
+  Value *BitSetGlobalOffset = ThenB.CreateLShr(
----------------
the code gen seems to generate the BT instruction sometimes. 

It's may be better to just create an instruction sequence that will be lowered to BT.
Example: 

void foo(unsigned char a, unsigned long b) {
  if (a & (1 << b))
    __builtin_trap();
}


   0:	0f a3 f7             	bt     %esi,%edi
   3:	72 01                	jb     6 <_Z3foohm+0x6>
   5:	c3                   	retq   
   6:	0f 0b                	ud2

http://reviews.llvm.org/D7288

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