[PATCH] D30270: [InstCombine] don't try SimplifyDemandedInstructionBits from add/sub because it's slow and unlikely to succeed

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
Herald added a subscriber: mcrosier.

Notably, no regression tests change when we remove these calls, and these are expensive calls.

The motivation comes from the general acknowledgement that the compiler is getting slower:
http://lists.llvm.org/pipermail/llvm-dev/2017-January/109188.html
http://lists.llvm.org/pipermail/llvm-dev/2016-December/108279.html

And specifically the test case attached to PR32037:
https://bugs.llvm.org//show_bug.cgi?id=32037

Profiling the middle-end (opt) part of the compile:
$ ./opt -O2 row_common.bc -o /dev/null

...visitAdd and visitSub are near the top of the instcombine list, and the calls to SimplifyDemandedInstructionBits() are high within each of those. Those calls account for 1%+ of the opt time in either debug or release profiles. And that's the rough win I see from this patch when testing opt built release from r295864 on an iMac with Haswell 4GHz (model 4790K).

It seems unlikely that we'd be able to eliminate add/sub or change their operands given that add/sub normally affect all bits, and the PR32037 example shows no IR difference after this change using -O2.

Also worth noting - the code comment in visitAdd:
// This handles stuff like (X & 254)+1 -> (X&254)|1
...isn't true. That transform is handled later with a call to haveNoCommonBitsSet().


https://reviews.llvm.org/D30270

Files:
  lib/Transforms/InstCombine/InstCombineAddSub.cpp


Index: lib/Transforms/InstCombine/InstCombineAddSub.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1078,11 +1078,6 @@
   // FIXME: Use the match above instead of dyn_cast to allow these transforms
   // for splat vectors.
   if (ConstantInt *CI = dyn_cast<ConstantInt>(RHS)) {
-    // See if SimplifyDemandedBits can simplify this.  This handles stuff like
-    // (X & 254)+1 -> (X&254)|1
-    if (SimplifyDemandedInstructionBits(I))
-      return &I;
-
     // zext(bool) + C -> bool ? C + 1 : C
     if (ZExtInst *ZI = dyn_cast<ZExtInst>(LHS))
       if (ZI->getSrcTy()->isIntegerTy(1))
@@ -1589,9 +1584,6 @@
     if (match(Op1, m_Add(m_Value(X), m_Constant(C2))))
       return BinaryOperator::CreateSub(ConstantExpr::getSub(C, C2), X);
 
-    if (SimplifyDemandedInstructionBits(I))
-      return &I;
-
     // Fold (sub 0, (zext bool to B)) --> (sext bool to B)
     if (C->isNullValue() && match(Op1, m_ZExt(m_Value(X))))
       if (X->getType()->getScalarType()->isIntegerTy(1))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30270.89402.patch
Type: text/x-patch
Size: 1122 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170222/f131154d/attachment.bin>