[PATCH] D53390: [DebugInfo][Dexter] Unreachable line stepped onto after SimplifyCFG.

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1379
+  Instruction *InsertPt = BIParent->size() > 1
+                              ? BIParent->getTerminator()->getPrevNode()
+                              : BIParent->getTerminator();
----------------
CarlosAlbertoEnciso wrote:
> CarlosAlbertoEnciso wrote:
> > vsk wrote:
> > > A bit nitty-gritty, but: Can the instruction before the terminator be a debug info intrinsic?
> > > 
> > > If so, I think the line location you pick be different if you switch from -gline-tables-only versus -g.
> > Thanks for your review.
> > 
> > Changing the test case to:
> > 
> > ```
> > int main() {
> >   volatile int foo = 0;
> > 
> >   int beards = 0;
> >   bool cond = foo == 4;
> >   if (cond)
> >     beards = 8;
> >   else
> >     beards = 4;
> > 
> >   volatile bool tmp;
> >   volatile bool *face = &tmp;
> >   *face = cond;
> >  
> >   return beards;
> > }
> > ```
> > 
> > The IR will look like:
> > 
> > ```
> >   ...
> >   %foo.0. = load volatile i32, i32* %foo, align 4, !dbg !28, !tbaa !23
> >   %cmp = icmp eq i32 %foo.0., 4, !dbg !28
> >   %frombool = zext i1 %cmp to i8, !dbg !28
> >   call void @llvm.dbg.value(metadata i8 %frombool, metadata !16, metadata !DIExpression()), !dbg !28
> >   br i1 %cmp, label %if.then, label %if.else, !dbg !29
> > 
> > if.then:                                          ; preds = %entry
> >   call void @llvm.dbg.value(metadata i32 8, metadata !15, metadata !DIExpression()), !dbg !27
> >   br label %if.end, !dbg !30
> > 
> > if.else:                                          ; preds = %entry
> >   call void @llvm.dbg.value(metadata i32 4, metadata !15, metadata !DIExpression()), !dbg !27
> >   br label %if.end
> > 
> > if.end:                                           ; preds = %if.else, %if.then
> > ...
> > ```
> > 
> > The instruction before the terminator is a debug info intrinsic.
> > 
> > 
> > If so, I think the line location you pick be different if you switch from -gline-tables-only versus -g.
> 
> IR for '-g'
> 
> ```
>   ...
>   %cmp = icmp eq i32 %foo.0., 4, !dbg !26
>   %frombool = zext i1 %cmp to i8, !dbg !26
>   call void @llvm.dbg.value(metadata i8 %frombool, metadata !16, metadata !DIExpression()), !dbg !26
>   br i1 %cmp, label %if.then, label %if.else, !dbg !27
>   ...
>   !26 = !DILocation(line: 5, scope: !8)
> ```
> 
> IR for '-gline-tables-only'
> 
> ```
>   ...
>   %cmp = icmp eq i32 %foo.0., 4, !dbg !15
>   %frombool = zext i1 %cmp to i8, !dbg !15
>   br i1 %cmp, label %if.then, label %if.else, !dbg !16
>   ...
>   !15 = !DILocation(line: 5, scope: !8)
>   ...
> ```
> InsertPts
> ```
>   call void @llvm.dbg.value(metadata i8 %frombool, metadata !16, metadata !DIExpression()), !dbg !26
> 
> &&
> 
>   %frombool = zext i1 %cmp to i8, !dbg !15
> ```
> 
> For both options, the line location of the insertion point is the same, debug location `!DILocation(line: 5, scope: !8)`.
> 
> 
I think it's still possible to construct cases where -g vs. -gline-tables-only produces different locations. What if you add `int bar = 0` on the line after `bool cond = foo == 4`? I see this:

```
  %6 = zext i1 %5 to i8, !dbg !31
  call void @llvm.dbg.value(metadata i8 %6, metadata !15, metadata !DIExpression()), !dbg !31
  call void @llvm.dbg.value(metadata i32 0, metadata !17, metadata !DIExpression()), !dbg !32
  br i1 %5, label %7, label %8, !dbg !33
```

Where !32 and !31 are on different lines. Link in godbolt: https://godbolt.org/z/VswzLx


https://reviews.llvm.org/D53390