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

[Peter Collingbourne]

Also implemented an optimization for bit sets of size <= 64 bits.


================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:168
@@ +167,3 @@
+LowerBitSets::BitSetInfo LowerBitSets::buildBitSet(
+    Module &M,
+    MDString *BitSet,
----------------
pcc wrote:
> kcc wrote:
> > do you need M here? 
> No, I'll remove it.
Done.

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

================
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
----------------
pcc wrote:
> kcc wrote:
> > 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.  
> Fair enough, I'll see if I can add a few unit tests.
Done.

================
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(
----------------
pcc wrote:
> kcc wrote:
> > 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    
> > 
> > 
> > 
> Yes, we can do that. I did try before to emit the register variant of BT but I guess I wasn't using the correct pattern. I'll play around with the IR to see if I can get it to match.
> 
> I was also thinking more of the memory variant of BT that basically does exactly what we want given a bit offset.
This now uses the register BT.

http://reviews.llvm.org/D7288

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