[PATCH] [InstCombine] Teach how to fold a select into a cttz/ctlz with the 'is_zero_undef' flag cleared.

[Pete Cooper]

Hi Andrea

Is this intended to replace the work in CodeGenPrepare:r225274?  

If you match this to a single intrinsic call in instcombine then it would be relatively simple for SimplifyCFG to then speculate it with existing code.  Then we can remove the code from CGP?

Thanks,
Pete
> On Jan 9, 2015, at 9:56 AM, David Majnemer <david.majnemer at gmail.com> wrote:
> 
> ================
> Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:465-468
> @@ +464,6 @@
> +  // Check that a constant is propagated when CmpLHS is zero.
> +  if (Pred == ICmpInst::ICMP_EQ && match(TrueVal, m_ConstantInt(CI)))
> +    Count = FalseVal;
> +  else if (Pred == ICmpInst::ICMP_NE && match(FalseVal, m_ConstantInt(CI)))
> +    Count = TrueVal;
> +  else
> ----------------
> `@llvm.cttz.*` and `@llvm.ctlz.*` both accept vector types as arguments.  You may want to switch from `m_ConstantInt` to `m_APInt` because it will match against a splatted `ConstantVector` as well.
> 
> You could go even further and use `m_SpecificInt(II->getType()->getScalarSizeInBits())` to save you a check later on.
> 
> ================
> Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:478-483
> @@ +477,8 @@
> +
> +  if (match(Count, m_Intrinsic<Intrinsic::cttz>(m_Value(V))) ||
> +      match(Count, m_Intrinsic<Intrinsic::ctlz>(m_Value(V)))) {
> +    // The value in input to the cttz/ctlz must be the same value in input to
> +    // the compare instruction.
> +    if (V != CmpLHS)
> +      return nullptr;
> +
> ----------------
> You could use `m_Specific(CmpLHS)` instead of `m_Value(V)` and `V != CmpLHS`.
> 
> ================
> Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:492-498
> @@ +491,9 @@
> +        // Explicitly set the 'undef_on_zero' flag to zero.
> +        Type *Ty = II->getArgOperand(0)->getType();
> +        Value *Args[] = { II->getArgOperand(0),
> +                          ConstantInt::getFalse(II->getContext()) };
> +        Module *M = II->getParent()->getParent()->getParent();
> +        Value *IF = Intrinsic::getDeclaration(M, II->getIntrinsicID(), Ty);
> +        CallInst *NewI = Builder.CreateCall(IF, Args);
> +        NewI->setTailCallKind(II->getTailCallKind());
> +        Count = NewI;
> ----------------
> I wonder if it might be nicer to clone `II` and then fixup the clone.
> Something like:
>  NewI = II->clone();
>  NewI->setOperand(1, Constant::getNullValue(II->getArgOperand(1)->getType()));
> 
> This formulation also has the advantage of working with vector types.
> 
> http://reviews.llvm.org/D6891
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits