[llvm] 678b9c5 - [InstCombine] try difference-of-shifts factorization before negator

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 24 12:09:53 PST 2020


Yes - this was the quick and easy fix for the motivating example in PR47430
(forgot to note that in the commit message), but we can go further.
I noticed the same/similar missing multiply-by-negative-power-of-2 fold
here:
https://bugs.llvm.org/show_bug.cgi?id=47430#c3

On Tue, Nov 24, 2020 at 2:34 PM Roman Lebedev <lebedev.ri at gmail.com> wrote:

> On Tue, Nov 24, 2020 at 9:56 PM Sanjay Patel via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> >
> >
> > Author: Sanjay Patel
> > Date: 2020-11-24T13:56:30-05:00
> > New Revision: 678b9c5dde0d119e91f5f905c3d9101cf8c514f9
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/678b9c5dde0d119e91f5f905c3d9101cf8c514f9
> > DIFF:
> https://github.com/llvm/llvm-project/commit/678b9c5dde0d119e91f5f905c3d9101cf8c514f9.diff
> >
> > LOG: [InstCombine] try difference-of-shifts factorization before negator
> >
> > We need to preserve wrapping flags to allow better folds.
> > The cases with geps may be non-intuitive, but that appears to agree with
> Alive2:
> > https://alive2.llvm.org/ce/z/JQcqw7
> > We create 'nsw' ops independent from the original wrapping on the sub.
> >
> > Added:
> >
> >
> > Modified:
> >     llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> >     llvm/test/Transforms/InstCombine/sub-gep.ll
> >     llvm/test/Transforms/InstCombine/sub.ll
> >
> > Removed:
> >
> >
> >
> >
> ################################################################################
> > diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> > index 9a6a790aefaf..bf356d0de94b 100644
> > --- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> > +++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> > @@ -1722,6 +1722,10 @@ Instruction
> *InstCombinerImpl::visitSub(BinaryOperator &I) {
> >      return Res;
> >    }
> >
> > +  // Try this before Negator to preserve NSW flag.
> > +  if (Instruction *R = factorizeMathWithShlOps(I, Builder))
> > +    return R;
> > +
> >    if (Constant *C = dyn_cast<Constant>(Op0)) {
> >      Value *X;
> >      Constant *C2;
> > @@ -1770,9 +1774,6 @@ Instruction
> *InstCombinerImpl::visitSub(BinaryOperator &I) {
> >    if (Value *V = SimplifyUsingDistributiveLaws(I))
> >      return replaceInstUsesWith(I, V);
> >
> > -  if (Instruction *R = factorizeMathWithShlOps(I, Builder))
> > -    return R;
> > -
> >    if (I.getType()->isIntOrIntVectorTy(1))
> >      return BinaryOperator::CreateXor(Op0, Op1);
> >
> >
> > diff  --git a/llvm/test/Transforms/InstCombine/sub-gep.ll
> b/llvm/test/Transforms/InstCombine/sub-gep.ll
> > index d6ace8275c10..bc18608da7a5 100644
> > --- a/llvm/test/Transforms/InstCombine/sub-gep.ll
> > +++ b/llvm/test/Transforms/InstCombine/sub-gep.ll
> > @@ -125,9 +125,8 @@ define i64 @test_inbounds2_nuw_swapped([0 x i32]*
> %base, i64 %idx) {
> >
> >  define i64 @test_inbounds_two_gep([0 x i32]* %base, i64 %idx, i64
> %idx2) {
>
> >  ; CHECK-LABEL: @test_inbounds_two_gep(
> > -; CHECK-NEXT:    [[P2_IDX:%.*]] = shl nsw i64 [[IDX2:%.*]], 2
> > -; CHECK-NEXT:    [[P1_IDX_NEG:%.*]] = mul i64 [[IDX:%.*]], -4
> > -; CHECK-NEXT:    [[GEPDIFF:%.*]] = add i64 [[P1_IDX_NEG]], [[P2_IDX]]
> > +; CHECK-NEXT:    [[TMP1:%.*]] = sub nsw i64 [[IDX2:%.*]], [[IDX:%.*]]
> > +; CHECK-NEXT:    [[GEPDIFF:%.*]] = shl nsw i64 [[TMP1]], 2
> >  ; CHECK-NEXT:    ret i64 [[GEPDIFF]]
> FWIW modulo the no-wrap flags we could/should be getting this already:
> https://rise4fun.com/Alive/ER4
>
> >  ;
> >    %p1 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64
> %idx
> > @@ -140,9 +139,8 @@ define i64 @test_inbounds_two_gep([0 x i32]* %base,
> i64 %idx, i64 %idx2) {
> >
> >  define i64 @test_inbounds_nsw_two_gep([0 x i32]* %base, i64 %idx, i64
> %idx2) {
> >  ; CHECK-LABEL: @test_inbounds_nsw_two_gep(
> > -; CHECK-NEXT:    [[P2_IDX:%.*]] = shl nsw i64 [[IDX2:%.*]], 2
> > -; CHECK-NEXT:    [[P1_IDX_NEG:%.*]] = mul i64 [[IDX:%.*]], -4
> > -; CHECK-NEXT:    [[GEPDIFF:%.*]] = add i64 [[P1_IDX_NEG]], [[P2_IDX]]
> > +; CHECK-NEXT:    [[TMP1:%.*]] = sub nsw i64 [[IDX2:%.*]], [[IDX:%.*]]
> > +; CHECK-NEXT:    [[GEPDIFF:%.*]] = shl nsw i64 [[TMP1]], 2
> >  ; CHECK-NEXT:    ret i64 [[GEPDIFF]]
> >  ;
> >    %p1 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64
> %idx
> > @@ -155,9 +153,8 @@ define i64 @test_inbounds_nsw_two_gep([0 x i32]*
> %base, i64 %idx, i64 %idx2) {
> >
> >  define i64 @test_inbounds_nuw_two_gep([0 x i32]* %base, i64 %idx, i64
> %idx2) {
> >  ; CHECK-LABEL: @test_inbounds_nuw_two_gep(
> > -; CHECK-NEXT:    [[P2_IDX:%.*]] = shl nsw i64 [[IDX2:%.*]], 2
> > -; CHECK-NEXT:    [[P1_IDX_NEG:%.*]] = mul i64 [[IDX:%.*]], -4
> > -; CHECK-NEXT:    [[GEPDIFF:%.*]] = add i64 [[P1_IDX_NEG]], [[P2_IDX]]
> > +; CHECK-NEXT:    [[TMP1:%.*]] = sub nsw i64 [[IDX2:%.*]], [[IDX:%.*]]
> > +; CHECK-NEXT:    [[GEPDIFF:%.*]] = shl nsw i64 [[TMP1]], 2
> >  ; CHECK-NEXT:    ret i64 [[GEPDIFF]]
> >  ;
> >    %p1 = getelementptr inbounds [0 x i32], [0 x i32]* %base, i64 0, i64
> %idx
> >
> > diff  --git a/llvm/test/Transforms/InstCombine/sub.ll
> b/llvm/test/Transforms/InstCombine/sub.ll
> > index f325f8d09cf9..066085fc2535 100644
> > --- a/llvm/test/Transforms/InstCombine/sub.ll
> > +++ b/llvm/test/Transforms/InstCombine/sub.ll
> > @@ -1505,9 +1505,8 @@ define <2 x i8>
> @sub_mask_lowbits_splat_extra_use(<2 x i8> %x, <2 x i8>* %p) {
> >
> >  define i16 @sub_nsw_mul_nsw(i16 %x, i16 %y) {
> >  ; CHECK-LABEL: @sub_nsw_mul_nsw(
> > -; CHECK-NEXT:    [[X8:%.*]] = shl nsw i16 [[X:%.*]], 3
> > -; CHECK-NEXT:    [[Y8_NEG:%.*]] = mul i16 [[Y:%.*]], -8
> > -; CHECK-NEXT:    [[R:%.*]] = add i16 [[Y8_NEG]], [[X8]]
> > +; CHECK-NEXT:    [[TMP1:%.*]] = sub nsw i16 [[X:%.*]], [[Y:%.*]]
> > +; CHECK-NEXT:    [[R:%.*]] = shl nsw i16 [[TMP1]], 3
> >  ; CHECK-NEXT:    ret i16 [[R]]
> >  ;
> >    %x8 = mul nsw i16 %x, 8
> > @@ -1518,9 +1517,8 @@ define i16 @sub_nsw_mul_nsw(i16 %x, i16 %y) {
> >
> >  define i16 @sub_nuw_mul_nsw(i16 %x, i16 %y) {
> >  ; CHECK-LABEL: @sub_nuw_mul_nsw(
> > -; CHECK-NEXT:    [[X8:%.*]] = shl nsw i16 [[X:%.*]], 2
> > -; CHECK-NEXT:    [[Y8_NEG:%.*]] = mul i16 [[Y:%.*]], -4
> > -; CHECK-NEXT:    [[R:%.*]] = add i16 [[Y8_NEG]], [[X8]]
> > +; CHECK-NEXT:    [[TMP1:%.*]] = sub i16 [[X:%.*]], [[Y:%.*]]
> > +; CHECK-NEXT:    [[R:%.*]] = shl i16 [[TMP1]], 2
> >  ; CHECK-NEXT:    ret i16 [[R]]
> >  ;
> >    %x8 = mul nsw i16 %x, 4
> > @@ -1531,9 +1529,8 @@ define i16 @sub_nuw_mul_nsw(i16 %x, i16 %y) {
> >
> >  define i16 @sub_mul_nsw(i16 %x, i16 %y) {
> >  ; CHECK-LABEL: @sub_mul_nsw(
> > -; CHECK-NEXT:    [[X8:%.*]] = shl nsw i16 [[X:%.*]], 4
> > -; CHECK-NEXT:    [[Y8_NEG:%.*]] = mul i16 [[Y:%.*]], -16
> > -; CHECK-NEXT:    [[R:%.*]] = add i16 [[Y8_NEG]], [[X8]]
> > +; CHECK-NEXT:    [[TMP1:%.*]] = sub i16 [[X:%.*]], [[Y:%.*]]
> > +; CHECK-NEXT:    [[R:%.*]] = shl i16 [[TMP1]], 4
> >  ; CHECK-NEXT:    ret i16 [[R]]
> >  ;
> >    %x8 = mul nsw i16 %x, 16
> > @@ -1544,9 +1541,8 @@ define i16 @sub_mul_nsw(i16 %x, i16 %y) {
> >
> >  define i16 @sub_nsw_mul_nuw(i16 %x, i16 %y) {
> >  ; CHECK-LABEL: @sub_nsw_mul_nuw(
> > -; CHECK-NEXT:    [[X8:%.*]] = shl nuw i16 [[X:%.*]], 3
> > -; CHECK-NEXT:    [[Y8_NEG:%.*]] = mul i16 [[Y:%.*]], -8
> > -; CHECK-NEXT:    [[R:%.*]] = add i16 [[Y8_NEG]], [[X8]]
> > +; CHECK-NEXT:    [[TMP1:%.*]] = sub i16 [[X:%.*]], [[Y:%.*]]
> > +; CHECK-NEXT:    [[R:%.*]] = shl i16 [[TMP1]], 3
> >  ; CHECK-NEXT:    ret i16 [[R]]
> >  ;
> >    %x8 = mul nuw i16 %x, 8
> > @@ -1557,9 +1553,8 @@ define i16 @sub_nsw_mul_nuw(i16 %x, i16 %y) {
> >
> >  define i16 @sub_nuw_mul_nuw(i16 %x, i16 %y) {
> >  ; CHECK-LABEL: @sub_nuw_mul_nuw(
> > -; CHECK-NEXT:    [[X8:%.*]] = shl nuw i16 [[X:%.*]], 4
> > -; CHECK-NEXT:    [[Y8_NEG:%.*]] = mul i16 [[Y:%.*]], -16
> > -; CHECK-NEXT:    [[R:%.*]] = add i16 [[Y8_NEG]], [[X8]]
> > +; CHECK-NEXT:    [[TMP1:%.*]] = sub nuw i16 [[X:%.*]], [[Y:%.*]]
> > +; CHECK-NEXT:    [[R:%.*]] = shl nuw i16 [[TMP1]], 4
> >  ; CHECK-NEXT:    ret i16 [[R]]
> >  ;
> >    %x8 = mul nuw i16 %x, 16
> > @@ -1570,9 +1565,8 @@ define i16 @sub_nuw_mul_nuw(i16 %x, i16 %y) {
> >
> >  define i16 @sub_mul_nuw(i16 %x, i16 %y) {
> >  ; CHECK-LABEL: @sub_mul_nuw(
> > -; CHECK-NEXT:    [[X8:%.*]] = shl nuw i16 [[X:%.*]], 5
> > -; CHECK-NEXT:    [[Y8_NEG:%.*]] = mul i16 [[Y:%.*]], -32
> > -; CHECK-NEXT:    [[R:%.*]] = add i16 [[Y8_NEG]], [[X8]]
> > +; CHECK-NEXT:    [[TMP1:%.*]] = sub i16 [[X:%.*]], [[Y:%.*]]
> > +; CHECK-NEXT:    [[R:%.*]] = shl i16 [[TMP1]], 5
> >  ; CHECK-NEXT:    ret i16 [[R]]
> >  ;
> >    %x8 = mul nuw i16 %x, 32
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201124/0dbf3fa0/attachment.html>


More information about the llvm-commits mailing list