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

Kostya Serebryany kcc at google.com
Fri Feb 6 15:11:44 PST 2015

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.

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

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



More information about the llvm-commits mailing list