[PATCH] D124503: [InstCombine] sub(add(X,Y),umin(Y,Z)) --> add(X,usub.sat(Y,Z))

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

A couple of minors



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2029
         return replaceInstUsesWith(I, InvMaxMin);
       }
+      // sub(add(X,Y),umin(Y,Z)) --> add(X,usub.sat(Y,Z))
----------------
(style) Move this inside braces (and the same for the new fold):
```
  {
      Value *X = II->getLHS();
      Value *Y = II->getRHS();
      if (match(Op0, m_c_Add(m_Specific(X), m_Specific(Y))) &&
          (Op0->hasOneUse() || Op1->hasOneUse())) {
        Intrinsic::ID InvID = getInverseMinMaxIntrinsic(II->getIntrinsicID());
        Value *InvMaxMin = Builder.CreateBinaryIntrinsic(InvID, X, Y);
        return replaceInstUsesWith(I, InvMaxMin);
      }
  }
```


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2036
+            Op1->hasOneUse()) {
+          if (Swap)
+            std::swap(X, Y);
----------------
Maybe move the "for (bool Swap : {false, true})" loop inside the match to avoid performing the match twice?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124503