[PATCH] D29254: Do not propagate DebugLoc across basic blocks

[Taewook Oh via Phabricator via llvm-commits]

twoh created this revision.

DebugLoc shouldn't be propagated across basic blocks to prevent incorrect stepping and imprecise sample profile result. https://reviews.llvm.org/rL288903 addressed the wrong DebugLoc propagation issue by limiting the copy of DebugLoc when GVN removes a fully redundant load that is dominated by some other load. However, DebugLoc is still incorrectly propagated in the following example:

1:  extern int g;
2: 
3:  void foo(int x, int y, int z) {
4:    if (x)
5:      g = 0;
6:    else
7:      g = 1;
8:
9:    int i = 0;
10:   for ( ; i < y ; i++)
11:     if (i > z)
12:       g++;
13: }

Below is LLVM IR representation of the program before GVN:

@g = external local_unnamed_addr global i32, align 4

; Function Attrs: nounwind uwtable
define void @foo(i32 %x, i32 %y, i32 %z) local_unnamed_addr #0 !dbg !4 {
entry:

  %not.tobool = icmp eq i32 %x, 0, !dbg !8
  %.sink = zext i1 %not.tobool to i32, !dbg !8
  store i32 %.sink, i32* @g, align 4, !tbaa !9
  %cmp8 = icmp sgt i32 %y, 0, !dbg !13
  br i1 %cmp8, label %for.body.preheader, label %for.end, !dbg !17

for.body.preheader:                               ; preds = %entry

  br label %for.body, !dbg !19

for.body:                                         ; preds = %for.body.preheader, %for.inc

  %i.09 = phi i32 [ %inc4, %for.inc ], [ 0, %for.body.preheader ]
  %cmp1 = icmp sgt i32 %i.09, %z, !dbg !19
  br i1 %cmp1, label %if.then2, label %for.inc, !dbg !21

if.then2:                                         ; preds = %for.body

  %0 = load i32, i32* @g, align 4, !dbg !22, !tbaa !9
  %inc = add nsw i32 %0, 1, !dbg !22
  store i32 %inc, i32* @g, align 4, !dbg !22, !tbaa !9
  br label %for.inc, !dbg !23

for.inc:                                          ; preds = %for.body, %if.then2

  %inc4 = add nuw nsw i32 %i.09, 1, !dbg !24
  %exitcond = icmp ne i32 %inc4, %y, !dbg !13
  br i1 %exitcond, label %for.body, label %for.end.loopexit, !dbg !17

for.end.loopexit:                                 ; preds = %for.inc

  br label %for.end, !dbg !26

for.end:                                          ; preds = %for.end.loopexit, %entry

  ret void, !dbg !26

}

where

!21 = !DILocation(line: 11, column: 9, scope: !15)
!22 = !DILocation(line: 12, column: 8, scope: !20)
!23 = !DILocation(line: 12, column: 7, scope: !20)
!24 = !DILocation(line: 10, column: 20, scope: !25)

And below is after GVN:

@g = external local_unnamed_addr global i32, align 4

define void @foo(i32 %x, i32 %y, i32 %z) local_unnamed_addr !dbg !4 {
entry:

  %not.tobool = icmp eq i32 %x, 0, !dbg !8
  %.sink = zext i1 %not.tobool to i32, !dbg !8
  store i32 %.sink, i32* @g, align 4, !tbaa !9
  %cmp8 = icmp sgt i32 %y, 0, !dbg !13
  br i1 %cmp8, label %for.body.preheader, label %for.end, !dbg !17

for.body.preheader:                               ; preds = %entry

  br label %for.body, !dbg !19

for.body:                                         ; preds = %for.inc, %for.body.preheader

  %0 = phi i32 [ %1, %for.inc ], [ %.sink, %for.body.preheader ], !dbg !21
  %i.09 = phi i32 [ %inc4, %for.inc ], [ 0, %for.body.preheader ]
  %cmp1 = icmp sgt i32 %i.09, %z, !dbg !19
  br i1 %cmp1, label %if.then2, label %for.inc, !dbg !22

if.then2:                                         ; preds = %for.body

  %inc = add nsw i32 %0, 1, !dbg !21
  store i32 %inc, i32* @g, align 4, !dbg !21, !tbaa !9
  br label %for.inc, !dbg !23

for.inc:                                          ; preds = %if.then2, %for.body

  %1 = phi i32 [ %inc, %if.then2 ], [ %0, %for.body ]
  %inc4 = add nuw nsw i32 %i.09, 1, !dbg !24
  %exitcond = icmp ne i32 %inc4, %y, !dbg !13
  br i1 %exitcond, label %for.body, label %for.end.loopexit, !dbg !17

for.end.loopexit:                                 ; preds = %for.inc

  br label %for.end, !dbg !26

for.end:                                          ; preds = %for.end.loopexit, %entry

  ret void, !dbg !26

}

As you see, GVN removes the load in if.then2 block and creates a phi instruction in for.body for it. The problem is that DebugLoc of remove load instruction is propagated to the newly created phi instruction, which is wrong. https://reviews.llvm.org/rL288903 cannot handle this case because ValuesPerBlock.size() is not 1 in this example when the load is removed.


https://reviews.llvm.org/D29254

Files:
  lib/Transforms/Scalar/GVN.cpp
  test/Transforms/GVN/debugloc.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29254.86187.patch
Type: text/x-patch
Size: 4581 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170128/fc3faa9a/attachment.bin>