[PATCH] D138814: [InstCombine] Combine a/lshr of add -> (a + b < a)

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/pr34349.ll:17
+; CHECK-NEXT:    [[ADD_NARROWED_OVERFLOW:%.*]] = icmp ult i7 [[TMP2]], [[TMP1]]
+; CHECK-NEXT:    [[V14:%.*]] = zext i1 [[ADD_NARROWED_OVERFLOW]] to i8
 ; CHECK-NEXT:    ret i8 [[V14]]
----------------
nikic wrote:
> This case doesn't look like a profitable transform. Do I understand correctly that the actual motivating case here is the case where the inputs are `zext`, and then this was later generalized based on reviewer feedback to use known bits instead?
> 
> For the zext case, this looks like an obviously desirable transform, but for the general case (where the truncs may not fold away) this is less clearly beneficial. I would personally restrict this to just zext unless we have specific motivation otherwise. (But I'm also not going to fight this if reviewers disagree.)
I indeed did it using known bits after a comment from @spatel:

> In the earlier version(s) of this patch (is there a functional diff here from D107552?), there were questions about how it would interact with SCEV and/or vectorization.
> 
> If we're focusing on the overflow-only/minimal pattern, then I think we should structure the matching as a known-bits/demanded-bits problem. Ie, instead of zexts, we might have `and` masks. Instead of a shift at the end, that might also be a mask op.

For me it's fine to do it with just zext but then I'm afraid we'd miss some profitable cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138814