[PATCH] D60483: [InstCombine] Handle usubo always overflow

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Looks good, consider landing as two commits.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:3940
 
 bool InstCombiner::OptimizeOverflowCheck(OverflowCheckFlavor OCF, Value *LHS,
                                          Value *RHS, Instruction &OrigI,
----------------
Hm, i should check if this can be used (or is already used?) for some of the implicit-conversion checks that aren't using overflow intrinsics, but plain ir..


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:3993
     if (OCF == OCF_UNSIGNED_SUB) {
-      if (willNotOverflowUnsignedSub(LHS, RHS, OrigI))
         return SetResult(Builder.CreateNUWSub(LHS, RHS), Builder.getFalse(),
----------------
And `willNotOverflowUnsignedSub()` is: (unless i'm looking at the wrong func?)
```
   bool willNotOverflowUnsignedSub(const Value *LHS, const Value *RHS,
                                   const Instruction &CxtI) const {
     return computeOverflowForUnsignedSub(LHS, RHS, &CxtI) ==
            OverflowResult::NeverOverflows;
   }
```
So this should ideally be:
patch 0 (nfc)
```
- if (willNotOverflowUnsignedSub(LHS, RHS, OrigI))
+ OverflowResult OR = computeOverflowForUnsignedSub(LHS, RHS, &OrigI);
+ if (OR == OverflowResult::NeverOverflows)
```
patch 1 - 
```
+ if (OR == OverflowResult::AlwaysOverflows)
+   return SetResult(Builder.CreateSub(LHS, RHS), Builder.getTrue(), true);
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60483