[PATCH] D138814: [InstCombine] Combine a/lshr of add -> uadd.with.overflow

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/shift-add.ll:437
 ; CHECK-LABEL: @lshr_2_add_zext_basic(
-; CHECK-NEXT:    [[ZEXT_A:%.*]] = zext i1 [[A:%.*]] to i2
-; CHECK-NEXT:    [[ZEXT_B:%.*]] = zext i1 [[B:%.*]] to i2
-; CHECK-NEXT:    [[ADD:%.*]] = add nuw i2 [[ZEXT_A]], [[ZEXT_B]]
-; CHECK-NEXT:    [[LSHR:%.*]] = lshr i2 [[ADD]], 1
+; CHECK-NEXT:    [[UADDO:%.*]] = call { i1, i1 } @llvm.uadd.with.overflow.i1(i1 [[A:%.*]], i1 [[B:%.*]])
+; CHECK-NEXT:    [[UADDO_OVERFLOW:%.*]] = extractvalue { i1, i1 } [[UADDO]], 1
----------------
spatel wrote:
> This is an awkward way to check if 2 bools are set.
> We're missing the reduction of boolean math to logic either way:
> https://alive2.llvm.org/ce/z/4dBQhx
I removed support for types <3 bits; should I leave the test case in?


================
Comment at: llvm/test/Transforms/InstCombine/shift-add.ll:599
-; CHECK-NEXT:    [[ADD:%.*]] = add nuw nsw i64 [[ZEXT_A]], [[ZEXT_B]]
-; CHECK-NEXT:    [[TMP1:%.*]] = lshr i64 [[ADD]], 32
 ; CHECK-NEXT:    ret i64 [[TMP1]]
----------------
spatel wrote:
> For anything but the i1/i2 case, we should convert the `ashr` to `lshr` (as happened here and the next test)? 
> So we could just bail out of the transform if the type doesn't have at least 3 bits (ignore the possibility of `ashr`).
Ah I didn't know that, I'll simplify the combine to only check lshr then. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138814