[PATCH] D72048: [InstCombine] Preserve nuw on sub of geps (PR44419)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D72048#1800331 <https://reviews.llvm.org/D72048#1800331>, @nikic wrote:

> I think this is still not quite right for the case where there are multiple GEP indexes. For example:
>
>   define i64 @test_inbounds_nuw_multi_index([0 x [2 x i32]]* %base, i64 %idx, i64 %idx2) {
>   ; CHECK-LABEL: @test_inbounds_nuw_multi_index(
>   ; CHECK-NEXT:    [[P2_IDX:%.*]] = shl nuw nsw i64 [[IDX:%.*]], 3
>   ; CHECK-NEXT:    [[P2_IDX1:%.*]] = shl nuw nsw i64 [[IDX2:%.*]], 2
>   ; CHECK-NEXT:    [[P2_OFFS2:%.*]] = add i64 [[P2_IDX]], [[P2_IDX1]]
>   ; CHECK-NEXT:    ret i64 [[P2_OFFS2]]
>   ;
>     %p1 = getelementptr inbounds [0 x [2 x i32]], [0 x [2 x i32]]* %base, i64 0, i64 0, i64 0
>     %p2 = getelementptr inbounds [0 x [2 x i32]], [0 x [2 x i32]]* %base, i64 0, i64 %idx, i64 %idx2
>     %i1 = ptrtoint i32* %p1 to i64
>     %i2 = ptrtoint i32* %p2 to i64
>     %d = sub nuw i64 %i2, %i1
>     ret i64 %d
>   }
>
>
> Let's say %idx=-1, %idx2=4, then the overall result is `8 * %idx1 + 4 * %idx2 = 8`, which is positive, even though one of the intermediate indexes is negative and as such can't use `shl nuw`. I think in this case the `nuw` can't be on any of the instructions (including also not the `add`). Right?


Sure.

  ----------------------------------------
  define i64 @test_inbounds_nuw_multi_index(* %base, i64 %idx, i64 %idx2) {
  %0:
    %p1 = gep inbounds * %base, 0 x i64 0, 8 x i64 0, 4 x i64 0
    %p2 = gep inbounds * %base, 0 x i64 0, 8 x i64 %idx, 4 x i64 %idx2
    %i1 = ptrtoint * %p1 to i64
    %i2 = ptrtoint * %p2 to i64
    %d = sub nuw i64 %i2, %i1
    ret i64 %d
  }
  =>
  define i64 @test_inbounds_nuw_multi_index(* %base, i64 %idx, i64 %idx2) {
  %0:
    %P2_IDX = shl nsw nuw i64 %idx, 3
    %P2_IDX1 = shl nsw nuw i64 %idx2, 2
    %P2_OFFS2 = add i64 %P2_IDX, %P2_IDX1
    ret i64 %P2_OFFS2
  }
  Transformation doesn't verify!
  ERROR: Target is more poisonous than source
  
  Example:
  * %base = pointer(non-local, block_id=1, offset=498773658766409739)
  i64 %idx = #x0322800007800000 (225883668936130560)
  i64 %idx2 = #xfa00000000000000 (18014398509481984000, -432345564227567616)
  
  Source:
  * %p1 = pointer(non-local, block_id=1, offset=498773658766409739)
  * %p2 = pointer(non-local, block_id=1, offset=576460753345183755)
  i64 %i1 = #x171bffffc3ffffff (1665205961213607935)
  i64 %i2 = #x182fffffffffffff (1742893055792381951)
  i64 %d = #x011400003c000000 (77687094578774016)
  
  Target:
  i64 %P2_IDX = #x191400003c000000 (1807069351489044480)
  i64 %P2_IDX1 = poison
  i64 %P2_OFFS2 = poison
  Source value: #x011400003c000000 (77687094578774016)
  Target value: poison
  
  Summary:
    0 correct transformations
    1 incorrect transformations
    0 errors

  ----------------------------------------
  define i64 @test_inbounds_nuw_multi_index(* %base, i64 %idx, i64 %idx2) {
  %0:
    %p1 = gep inbounds * %base, 0 x i64 0, 8 x i64 0, 4 x i64 0
    %p2 = gep inbounds * %base, 0 x i64 0, 8 x i64 %idx, 4 x i64 %idx2
    %i1 = ptrtoint * %p1 to i64
    %i2 = ptrtoint * %p2 to i64
    %d = sub nuw i64 %i2, %i1
    ret i64 %d
  }
  =>
  define i64 @test_inbounds_nuw_multi_index(* %base, i64 %idx, i64 %idx2) {
  %0:
    %P2_IDX = shl nsw i64 %idx, 3
    %P2_IDX1 = shl nsw i64 %idx2, 2
    %P2_OFFS2 = add i64 %P2_IDX, %P2_IDX1
    ret i64 %P2_OFFS2
  }
  Transformation seems to be correct!
  
  Summary:
    1 correct transformations
    0 incorrect transformations
    0 errors


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

https://reviews.llvm.org/D72048