search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.48k stars 11.77k forks source link

GVNSink produces incorrect codegen with respect to GEPs #85333

Closed PiJoules closed 5 months ago

PiJoules commented 7 months ago 
target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
target triple = "thumbv7em-none-unknown-eabi"

%"struct.std::pair" = type <{ i32, %struct.substruct, [2 x i8] }>
%struct.substruct = type { i8, i8 }
%"struct.std::random_access_iterator_tag" = type { i8 }

define linkonce_odr dso_local void @__advance(ptr noundef nonnull align 4 dereferenceable(4) %__i, i32 noundef %__n) local_unnamed_addr {
entry:
  %0 = call i1 @llvm.is.constant.i32(i32 %__n)
  %cmp = icmp eq i32 %__n, 1
  %or.cond = and i1 %0, %cmp
  br i1 %or.cond, label %if.then, label %if.else

if.then:                                          ; preds = %entry
  %1 = load ptr, ptr %__i, align 4
  %incdec.ptr = getelementptr inbounds i8, ptr %1, i32 8
  store ptr %incdec.ptr, ptr %__i, align 4
  br label %if.end6

if.else:                                          ; preds = %entry
  %2 = call i1 @llvm.is.constant.i32(i32 %__n)
  %cmp2 = icmp eq i32 %__n, -1
  %or.cond7 = and i1 %2, %cmp2
  br i1 %or.cond7, label %if.then3, label %if.else5

if.then3:                                         ; preds = %if.else
  %3 = load ptr, ptr %__i, align 4
  %incdec.ptr4 = getelementptr inbounds i8, ptr %3, i32 -8
  store ptr %incdec.ptr4, ptr %__i, align 4
  br label %if.end6

if.else5:                                         ; preds = %if.else
  %4 = load ptr, ptr %__i, align 4
  %add.ptr = getelementptr inbounds %"struct.std::pair", ptr %4, i32 %__n
  store ptr %add.ptr, ptr %__i, align 4
  br label %if.end6

if.end6:                                          ; preds = %if.then3, %if.else5, %if.then
  ret void
}

The above IR when piped through gvn-sink via ./bin/opt < /tmp/repro.ll -passes=gvn-sink -S produces:

; ModuleID = '<stdin>'
source_filename = "<stdin>"
target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
target triple = "thumbv7em-none-unknown-eabi"

define linkonce_odr dso_local void @__advance(ptr noundef nonnull align 4 dereferenceable(4) %__i, i32 noundef %__n) local_unnamed_addr {
entry:
  %0 = call i1 @llvm.is.constant.i32(i32 %__n)
  %cmp = icmp eq i32 %__n, 1
  %or.cond = and i1 %0, %cmp
  br i1 %or.cond, label %if.then, label %if.else

if.then:                                          ; preds = %entry
  br label %if.end6

if.else:                                          ; preds = %entry
  %1 = call i1 @llvm.is.constant.i32(i32 %__n)
  %cmp2 = icmp eq i32 %__n, -1
  %or.cond7 = and i1 %1, %cmp2
  br i1 %or.cond7, label %if.then3, label %if.else5

if.then3:                                         ; preds = %if.else
  br label %if.end6

if.else5:                                         ; preds = %if.else
  br label %if.end6

if.end6:                                          ; preds = %if.else5, %if.then3, %if.then
  %.sink1 = phi i32 [ 8, %if.then ], [ -8, %if.then3 ], [ %__n, %if.else5 ]
  %2 = load ptr, ptr %__i, align 4
  %incdec.ptr = getelementptr inbounds i8, ptr %2, i32 %.sink1
  store ptr %incdec.ptr, ptr %__i, align 4
  ret void
}

; Function Attrs: convergent nocallback nofree nosync nounwind willreturn memory(none)
declare i1 @llvm.is.constant.i32(i32) #0

attributes #0 = { convergent nocallback nofree nosync nounwind willreturn memory(none) }

The GEP in the result is incorrect. Prior, the GEPs were

  %incdec.ptr = getelementptr inbounds i8, ptr %1, i32 8
  %incdec.ptr4 = getelementptr inbounds i8, ptr %3, i32 -8
  %add.ptr = getelementptr inbounds %"struct.std::pair", ptr %4, i32 %__n

but now it's

%incdec.ptr = getelementptr inbounds i8, ptr %2, i32 %.sink1

This results in an incorrect offset for %add.ptr = getelementptr inbounds %"struct.std::pair", ptr %4, i32 %__n since we'd now just add %.sink1 bytes (where %.sink1 = %__n which is the second argument), whereas before we'd add %__n * 8 bytes (where 8 is the sizeof %"struct.std::pair").

The before ASM is:

__advance:
        ldr     r2, [r0]
        add.w   r1, r2, r1, lsl #3
        str     r1, [r0]
        bx      lr

but the after asm is:

__advance:
        ldr     r2, [r0]
        add     r1, r2
        str     r1, [r0]
        bx      lr

The GEP should not be a candidate for sinking.

llvmbot commented 7 months ago

@llvm/issue-subscribers-bug

Author: None (PiJoules)

``` target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" target triple = "thumbv7em-none-unknown-eabi" %"struct.std::pair" = type <{ i32, %struct.substruct, [2 x i8] }> %struct.substruct = type { i8, i8 } %"struct.std::random_access_iterator_tag" = type { i8 } define linkonce_odr dso_local void @__advance(ptr noundef nonnull align 4 dereferenceable(4) %__i, i32 noundef %__n) local_unnamed_addr { entry: %0 = call i1 @llvm.is.constant.i32(i32 %__n) %cmp = icmp eq i32 %__n, 1 %or.cond = and i1 %0, %cmp br i1 %or.cond, label %if.then, label %if.else if.then: ; preds = %entry %1 = load ptr, ptr %__i, align 4 %incdec.ptr = getelementptr inbounds i8, ptr %1, i32 8 store ptr %incdec.ptr, ptr %__i, align 4 br label %if.end6 if.else: ; preds = %entry %2 = call i1 @llvm.is.constant.i32(i32 %__n) %cmp2 = icmp eq i32 %__n, -1 %or.cond7 = and i1 %2, %cmp2 br i1 %or.cond7, label %if.then3, label %if.else5 if.then3: ; preds = %if.else %3 = load ptr, ptr %__i, align 4 %incdec.ptr4 = getelementptr inbounds i8, ptr %3, i32 -8 store ptr %incdec.ptr4, ptr %__i, align 4 br label %if.end6 if.else5: ; preds = %if.else %4 = load ptr, ptr %__i, align 4 %add.ptr = getelementptr inbounds %"struct.std::pair", ptr %4, i32 %__n store ptr %add.ptr, ptr %__i, align 4 br label %if.end6 if.end6: ; preds = %if.then3, %if.else5, %if.then ret void } ``` The above IR when piped through gvn-sink via `./bin/opt < /tmp/repro.ll -passes=gvn-sink -S` produces: ``` ; ModuleID = '<stdin>' source_filename = "<stdin>" target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" target triple = "thumbv7em-none-unknown-eabi" define linkonce_odr dso_local void @__advance(ptr noundef nonnull align 4 dereferenceable(4) %__i, i32 noundef %__n) local_unnamed_addr { entry: %0 = call i1 @llvm.is.constant.i32(i32 %__n) %cmp = icmp eq i32 %__n, 1 %or.cond = and i1 %0, %cmp br i1 %or.cond, label %if.then, label %if.else if.then: ; preds = %entry br label %if.end6 if.else: ; preds = %entry %1 = call i1 @llvm.is.constant.i32(i32 %__n) %cmp2 = icmp eq i32 %__n, -1 %or.cond7 = and i1 %1, %cmp2 br i1 %or.cond7, label %if.then3, label %if.else5 if.then3: ; preds = %if.else br label %if.end6 if.else5: ; preds = %if.else br label %if.end6 if.end6: ; preds = %if.else5, %if.then3, %if.then %.sink1 = phi i32 [ 8, %if.then ], [ -8, %if.then3 ], [ %__n, %if.else5 ] %2 = load ptr, ptr %__i, align 4 %incdec.ptr = getelementptr inbounds i8, ptr %2, i32 %.sink1 store ptr %incdec.ptr, ptr %__i, align 4 ret void } ; Function Attrs: convergent nocallback nofree nosync nounwind willreturn memory(none) declare i1 @llvm.is.constant.i32(i32) #0 attributes #0 = { convergent nocallback nofree nosync nounwind willreturn memory(none) } ``` The GEP in the result is incorrect. Prior, the GEPs were ``` %incdec.ptr = getelementptr inbounds i8, ptr %1, i32 8 %incdec.ptr4 = getelementptr inbounds i8, ptr %3, i32 -8 %add.ptr = getelementptr inbounds %"struct.std::pair", ptr %4, i32 %__n ``` but now it's ``` %incdec.ptr = getelementptr inbounds i8, ptr %2, i32 %.sink1 ``` This results in an incorrect offset for `%add.ptr = getelementptr inbounds %"struct.std::pair", ptr %4, i32 %__n` since we'd now just add `%.sink1` bytes (where `%.sink1 = %__n` which is the second argument), whereas before we'd add `%__n * 8` bytes (where 8 is the sizeof `%"struct.std::pair"`). The before ASM is: ``` __advance: ldr r2, [r0] add.w r1, r2, r1, lsl #3 str r1, [r0] bx lr ``` but the after asm is: ``` __advance: ldr r2, [r0] add r1, r2 str r1, [r0] bx lr ``` The GEP should not be a candidate for sinking.