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

[spatel]

Summary

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().

[efriedma]

It looks like in the current version of SimplifyDemandedUseBits, simplifying the demanded bits on an Add with an all-ones mask is equivalent to calling computeKnownBits. That explains why you didn't find any regressions. :)

Like you've said, I can't see any reason why we'd want to do this; LGTM.

[davide]

Agree with Eli. LGTM.

llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp

if (Val->isNegative() &&
           ConstantInt::get(X->getType(), *C + Val->trunc(C->getBitWidth()));
       return new ZExtInst(Builder->CreateNUWAdd(X, NewC), I.getType());
     }
   }
 
   // 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))
         return SelectInst::Create(ZI->getOperand(0), AddOne(CI), CI);
 
     Value *XorLHS = nullptr; ConstantInt *XorRHS = nullptr;
     if (match(LHS, m_Xor(m_Value(XorLHS), m_ConstantInt(XorRHS)))) {
       uint32_t TySizeBits = I.getType()->getScalarSizeInBits();
if (SelectInst *SI = dyn_cast<SelectInst>(Op1))
       if (Instruction *R = FoldOpIntoSelect(I, SI))
         return R;
 
     // C-(X+C2) --> (C-C2)-X
     Constant *C2;
     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))
         return CastInst::CreateSExtOrBitCast(X, Op1->getType());
 
     // Fold (sub 0, (sext bool to B)) --> (zext bool to B)
     if (C->isNullValue() && match(Op1, m_SExt(m_Value(X))))
       if (X->getType()->getScalarType()->isIntegerTy(1))