[PATCH] D51976: [DebugInfo][Dexter] Speculated BB presents illegal variable value to debugger

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

Thanks, this is looking pretty close. A few comments inline.



================
Comment at: include/llvm/Transforms/Utils/Local.h:446
 
+/// Remove the debug intrinsic instructions for the given machine instruction.
+void dropDebugUsers(Instruction &I);
----------------
Please omit 'machine' here, as MachineInstr and Instruction are distinct.


================
Comment at: test/CodeGen/X86/pr38763.ll:36
+; CHECK:  %sub = add nsw i32 %foo.0.4, -2, !dbg !32
+; CHECK:  %result.0 = select i1 %cmp, i32 %add, i32 %sub, !dbg !34
+; CHECK:  call void @llvm.dbg.value(metadata i32 %result.0, metadata !16, metadata !DIExpression()), !dbg !27
----------------
For the test to be stringent, it needs to check that there these dbg.value's don't appear:

```
dbg.value(%add, !16, ...)
dbg.value(%sub, !16, ...)
```

One way to do that is to require exact next-line matches. E.g:

```
CHECK: %add = ...
CHECK-NOT: dbg.value(metadata i32 %add
CHECK-NEXT: %sub = ...
CHECK-NOT: dbg.value(metadata i32 %sub
CHECK-NEXT: %result.0 = select ...
CHECK-NEXT: d.v(%result.0, ...)
```


================
Comment at: test/CodeGen/X86/pr38763.ll:86
+; Function Attrs: argmemonly nounwind
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) #1
+
----------------
Are these lifetime markers needed to reproduce the issue?


================
Comment at: test/CodeGen/X86/pr38763.ll:93
+attributes #1 = { argmemonly nounwind }
+attributes #2 = { nounwind readnone speculatable }
+
----------------
Is the test still effective if you strip out all these attributes (i.e, remove '#0' etc from the file)? If so, doing that is a good way to make the test more maintainable.


https://reviews.llvm.org/D51976