[PATCH] D42536: [AggresiveInstCombine] Added support of select and ShuffleVector to TruncInstCombine expression pattern

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

I apologize for not noticing this before, but do all of the tests here fail in regular instcombine because there's an instruction that uses the same value more than once? Or are there more complicated patterns?

If it's just the simpler case (or even if it's not), can we enhance instcombine to catch those cases like this:

  Index: lib/Transforms/InstCombine/InstCombineCasts.cpp
  ===================================================================
  --- lib/Transforms/InstCombine/InstCombineCasts.cpp	(revision 323427)
  +++ lib/Transforms/InstCombine/InstCombineCasts.cpp	(working copy)
  @@ -184,8 +184,14 @@
     case Instruction::Shl:
     case Instruction::UDiv:
     case Instruction::URem: {
  -    Value *LHS = EvaluateInDifferentType(I->getOperand(0), Ty, isSigned);
  -    Value *RHS = EvaluateInDifferentType(I->getOperand(1), Ty, isSigned);
  +    Value *LHS, *RHS;
  +    if (I->getOperand(0) == I->getOperand(1)) {
  +      // Don't create an unnecessary value if the operands are repeated.
  +      LHS = RHS = EvaluateInDifferentType(I->getOperand(0), Ty, isSigned);
  +    } else {
  +      LHS = EvaluateInDifferentType(I->getOperand(0), Ty, isSigned);
  +      RHS = EvaluateInDifferentType(I->getOperand(1), Ty, isSigned);
  +    }
       Res = BinaryOperator::Create((Instruction::BinaryOps)Opc, LHS, RHS);
       break;
     }
  @@ -315,9 +321,12 @@
         I->getOperand(0)->getType() == Ty)
       return true;
   
  -  // We can't extend or shrink something that has multiple uses: doing so would
  -  // require duplicating the instruction in general, which isn't profitable.
  -  if (!I->hasOneUse()) return false;
  +  // We can't extend or shrink something that has multiple uses -- unless those
  +  // multiple uses are all in the same instruction -- doing so would require
  +  // duplicating the instruction which isn't profitable.
  +  if (!I->hasOneUse())
  +    if (any_of(I->users(), [&](User *U) { return U != I->user_back(); }))
  +      return false;
   
     unsigned Opc = I->getOpcode();
     switch (Opc) {


https://reviews.llvm.org/D42536