[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 inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2038
+    for (bool Swap : {false, true}) {
+      if (!isa<MinMaxIntrinsic>(Op1))
+        break;
----------------
Chenbing.Zheng wrote:
> RKSimon wrote:
> > I meant - can you incorporate these tests under the existing dyn_cast<MinMaxIntrinsic> check at Line 2021?
> I try to do it, but the code doesn't get much cleaner and it breaks readability.
At the very least the isa<MinMaxIntrinsic>(Op1) check should be at the top, not inside a loop in which its invariant.
```
if (isa<MinMaxIntrinsic>(Op1)) {
  Value *X, *Y, *Z;
  Type *Ty = I.getType();
  for (bool Swap : {false, true}) {
    ....
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124503