[PATCH] D89317: [InstructionSimplify] icmp simplification

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

1. Please upload with context: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
2. There should be at least 4 positive tests to exercise each of the commuted pattern paths. Vary the types (include a vector test) for more coverage/functionality. Do we have negative tests for when `nsw` is on the wrong `add` or the predicate is not `slt`?
3. I don't like the way the code around here is structured already, and this patch is making it more complicated. How about something like this:

  static bool trySimplifyICmpWithAdds(CmpInst::Predicate Pred, Value *LHS,
                                      Value *RHS) {
    // Canonicalize nsw add as RHS.
    if (!match(RHS, m_NSWAdd(m_Value(), m_Value())))
      std::swap(LHS, RHS);
    if (!match(RHS, m_NSWAdd(m_Value(), m_Value())))
      return false;
  
    Value *X;
    const APInt *C1, *C2;
    if (!match(LHS, m_c_Add(m_Value(X), m_APInt(C1))) ||
        !match(RHS, m_c_Add(m_Specific(X), m_APInt(C2))))
      return false;
  
    return C1->slt(*C2) && C1->isNonNegative();
  }


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89317/new/

https://reviews.llvm.org/D89317