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

[Nikita Popov via Phabricator via llvm-commits]

nikic marked 2 inline comments as done.
nikic added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/sub-gep.ll:30-41
 define i64 @test_nuw([0 x i32]* %base, i64 %idx) {
 ; CHECK-LABEL: @test_nuw(
-; CHECK-NEXT:    [[P2_IDX:%.*]] = shl i64 [[IDX:%.*]], 2
+; CHECK-NEXT:    [[P2_IDX:%.*]] = shl nuw i64 [[IDX:%.*]], 2
 ; CHECK-NEXT:    ret i64 [[P2_IDX]]
 ;
   %p1 = getelementptr [0 x i32], [0 x i32]* %base, i64 0, i64 0
   %p2 = getelementptr [0 x i32], [0 x i32]* %base, i64 0, i64 %idx
----------------
lebedev.ri wrote:
> ```
> ----------------------------------------
> define i64 @test_nuw(* %base, i64 %idx) {
> %0:
>   %p1 = gep * %base, 0 x i64 0, 4 x i64 0
>   %p2 = gep * %base, 0 x i64 0, 4 x i64 %idx
>   %i1 = ptrtoint * %p1 to i64
>   %i2 = ptrtoint * %p2 to i64
>   %d = sub nuw i64 %i2, %i1
>   ret i64 %d
> }
> =>
> define i64 @test_nuw(* %base, i64 %idx) {
> %0:
>   %P2_IDX = shl nuw i64 %idx, 2
>   ret i64 %P2_IDX
> }
> Transformation doesn't verify!
> ERROR: Target is more poisonous than source
> 
> Example:
> * %base = null
> i64 %idx = undef
> 
> Source:
> * %p1 = null
> * %p2 = null    [based on undef value]
> i64 %i1 = #x0000000000000000 (0)
> i64 %i2 = #x0000000000000000 (0)
> i64 %d = #x0000000000000000 (0)
> 
> Target:
> i64 %P2_IDX = poison
> Source value: #x0000000000000000 (0)
> Target value: poison
> 
> Summary:
>   0 correct transformations
>   1 incorrect transformations
>   0 errors
> 
> ```
Thanks for catching this! To give a more specific example, if %base=0 and %idx has only the top bit set, then %p1 and %p2 will both be zero and the original sub is nuw, but shifting the %idx is not nuw.


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

https://reviews.llvm.org/D72048