[InstCombine] visitTrunc - remove dead trunc(lshr (zext A), C) combine

[RKSimon]

Summary

I added additional test coverage at rG7a55989dc4305 - but all are handled independently of this combine and http://lab.llvm.org:8080/coverage/coverage-reports/ indicates the code is never used.

[lebedev.ri]

SGTM, but please add NFCI to patch's subj to better signal that this really shouldn't cause changes.

[xbolva00]

Binary changes

https://llvm-compile-time-tracker.com/compare.php?from=ccbb9827db4c30c93b92a204aeb2b98f9f3a723a&to=b610d73b3fe67fe6b693740dfac5fd21a60b1e44&stat=size-text

[lebedev.ri]

Clearly, as per tests, the motivational patterns are handled elsewhere.

In D88492#2302475, @xbolva00 wrote:

Binary changes

https://llvm-compile-time-tracker.com/compare.php?from=ccbb9827db4c30c93b92a204aeb2b98f9f3a723a&to=b610d73b3fe67fe6b693740dfac5fd21a60b1e44&stat=size-text

Much like with all of instcombine, i believe this heavily depends on the visitation order.
Which i guess means, this (used to) catch some unusual patterns, that didn't go through the usual def-simplification yet.
Probably lack of this fold causes some other followup folds to fire, or in different order, thus causing different final IR.

[nikic]

FWIW, when removing a redundant fold, it would be helpful to point out which other fold already handles it in the review summary.

[RKSimon]

For the record - the tests from rG7a55989dc430 are all being caught by a mixture of canEvaluateTruncated inside visitTrunc and the "lshr (zext iM X to iN), C --> zext (lshr X, C) to iN" fold in visitLShr - the bin diffs can probably be attributed to visitTrunc no longer catching it and having to wait for the inner visitLShr call.

BTW - All of these folds are still very weak on non-uniform vector support - the number of places where we're using m_ConstantInt or m_APInt instead of a more general m_Constant eval is really annoying.

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

if (match(Src, m_OneUse(m_c_Or(m_LShr(m_Value(X), m_Constant(C)),
       Constant *One = ConstantInt::get(SrcTy, APInt(SrcWidth, 1));
       Constant *MaskC = ConstantExpr::getShl(One, C);
       MaskC = ConstantExpr::getOr(MaskC, One);
       Value *And = Builder.CreateAnd(X, MaskC);
       return new ICmpInst(ICmpInst::ICMP_NE, And, Zero);
     }
   }
 
-  // FIXME: Maybe combine the next two transforms to handle the no cast case
-  // more efficiently. Support vector types. Cleanup code by using m_OneUse.
-
-  // Transform trunc(lshr (zext A), Cst) to eliminate one type conversion.
-  Value *A = nullptr;
-  if (Src->hasOneUse() &&
-      match(Src, m_LShr(m_ZExt(m_Value(A)), m_ConstantInt(Cst)))) {
-    // We have three types to worry about here, the type of A, the source of
-    // the truncate (MidSize), and the destination of the truncate. We know that
-    // ASize < MidSize   and MidSize > ResultSize, but don't know the relation
-    // between ASize and ResultSize.
-    unsigned ASize = A->getType()->getPrimitiveSizeInBits();
-
-    // If the shift amount is larger than the size of A, then the result is
-    // known to be zero because all the input bits got shifted out.
-    if (Cst->getZExtValue() >= ASize)
-      return replaceInstUsesWith(Trunc, Constant::getNullValue(DestTy));
-
-    // Since we're doing an lshr and a zero extend, and know that the shift
-    // amount is smaller than ASize, it is always safe to do the shift in A's
-    // type, then zero extend or truncate to the result.
-    Value *Shift = Builder.CreateLShr(A, Cst->getZExtValue());
-    Shift->takeName(Src);
-    return CastInst::CreateIntegerCast(Shift, DestTy, false);
-  }
-
+  Value *A;
   Constant *C;
   if (match(Src, m_LShr(m_SExt(m_Value(A)), m_Constant(C)))) {
     unsigned AWidth = A->getType()->getScalarSizeInBits();
     unsigned MaxShiftAmt = SrcWidth - std::max(DestWidth, AWidth);
     auto *OldSh = cast<Instruction>(Src);
     bool IsExact = OldSh->isExact();
 
     // If the shift is small enough, all zero bits created by the shift are