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

JF Bastien jfb at chromium.org
Thu Feb 19 13:59:41 PST 2015


================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:236
@@ +235,3 @@
+                                      Value *BitOffset) {
+  if (BSI.Bits.size() <= 8) {
+    // If the bit set is sufficiently small, we can avoid a load by bit testing
----------------
pcc wrote:
> jfb wrote:
> > pcc wrote:
> > > jfb wrote:
> > > > I'm mildly disappointed that the optimizer doesn't do this by taking into account ISA-specific sizes (and then removing the dead global because its address isn't taken).
> > > This should in principle be possible, but this pass runs late so in any case it seems best to directly generate the IR we need.
> > Does it need to run that late? 8 may not be the right number on all architectures, and you should see a good binary size reduction by GC'ing unreferenced globals (especially if CFI+devirtualization occurs).
> It seemed better to run the pass later because it splits basic blocks, which could pessimize things in other passes, and bitset creation essentially locks in the set of virtual tables that appear in the binary, preventing them from being GCd.
> 
> I'm not sure what the best way to deal with this might be. We might later want to split this pass into an early pass and a late pass, where the early pass uses bitset metadata to do devirtualization, a later globalopt pass GCs unused vtables and the late pass builds the actual bitsets.
Maybe I have a naive view, but shouldn't bitsets be GC'able if no test refers to them? This doesn't need to be an optimization that's specific to your code, LLVM can do this in general when a global doesn't escape and isn't address-taken (and in your case, is read-only). If this is correct, then I don't think you need to split up this pass, though I agree that you may want to do devirtualization earlier to expose more optimization opportunities.

Under the current setup, do redundant tests in the same function get eliminated and control flow merged?

This may be something that we can leave open for later changes: I think the current code is good in that it does what's required and is pretty efficient at it. I don't think the design will change substantially, but I do think there are further optimization opportunities here. WDYT?

http://reviews.llvm.org/D7288

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






More information about the llvm-commits mailing list