[PATCH] D24700: [InstCombine] optimize unsigned icmp of inc/dec like signed

[Matti Niemenmaa via llvm-commits]

Deewiant added a comment.

In https://reviews.llvm.org/D24700#546763, @spatel wrote:

> Not sure if this is easier to read, but if the existing code was rewritten with something like this:
>
>   auto getNewPred = [](CmpInst::Predicate P, Value *AddConstant) {
>     if (!match(AddConstant, m_AllOnes()) && !match(AddConstant, m_One()))
>       return ICmpInst::BAD_ICMP_PREDICATE;
>   
>     switch (P) {
>     case CmpInst::ICMP_SLT: return CmpInst::ICMP_SLE;
>     case CmpInst::ICMP_SGE: return CmpInst::ICMP_SGT;
>     case CmpInst::ICMP_SLE: return CmpInst::ICMP_SLT;
>     case CmpInst::ICMP_SGT: return CmpInst::ICMP_SGE;
>     default: return CmpInst::BAD_ICMP_PREDICATE;
>     }
>   };
>   
>
> Then your patch would just be to add some cases to the switch?


If I understood your meaning, I think using just a function like that would result in this matching too many cases. E.g. if the usage would be something like:

  const auto NewPred = getNewPred(Pred);
  if (A && NoOp0WrapProblem)
    return new ICmpInst(NewPred, A, Op1);

We'd be performing incorrect transformations like `icmp sge (X + 1), Y -> icmp sgt X, Y`. So we'd still have to combine specific predicates with specific `match` calls (and non-null `A`/`C` checks) somehow, and at that point I don't think that would really improve readability.


https://reviews.llvm.org/D24700