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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

There's a potentially missing/difficult optimization in the larger example. It boils down to this:
https://alive2.llvm.org/ce/z/qpCq-X

Ie, should we replace a value (the trunc) with a narrow math op that produces the identical value directly? That might be good because it removes a use of the wide add and increases parallelism, but it might be bad because it creates an independent math op that could impede analysis and be more expensive than a trunc in codegen. That's the problem shown in issue #59217 <https://github.com/llvm/llvm-project/issues/59217> - with a mul, it's pretty clear that we don't want to create more math.

Given that there's no clear answer (and no way to invert the transform that I'm aware of), the direction of this patch is ok with me.



================
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
----------------
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


================
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]]
----------------
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`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138814