[PATCH] D81939: [deadargelim] Attach dbg info to the insert/extractvalue instructions

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro marked 3 inline comments as done.
djtodoro added a comment.

@aprantl @vsk Thanks a lot for the feedback!



================
Comment at: llvm/test/DebugInfo/X86/dbgloc-insert-extract-val-instrs.ll:4
+
+; RUN: opt < %s -deadargelim -check-debugify -S 2>&1 \
+; RUN:     | FileCheck %s -check-prefix=DEBUG
----------------
vsk wrote:
> It doesn't look like the -check-debugify output is important, so it shouldn't be necessary to run the pass.
> 
> Also, since the dbg.values are also not important, please run -debugify with -debugify-level=locations to omit those intrinsics.
I see..thanks :)


================
Comment at: llvm/test/DebugInfo/X86/dbgloc-insert-extract-val-instrs.ll:5
+; RUN: opt < %s -deadargelim -check-debugify -S 2>&1 \
+; RUN:     | FileCheck %s -check-prefix=DEBUG
+
----------------
aprantl wrote:
> Why use a custom prefix if there is only one FileCheck invocation?
Oh, sure..


================
Comment at: llvm/test/DebugInfo/X86/dbgloc-insert-extract-val-instrs.ll:7
+
+; DEBUG: %oldret = extractvalue { i16, i32 } %B, 1, !dbg ![[RET1:.*]]
+; DEBUG: %oldret = extractvalue { i32, i16 } %B, 0, !dbg ![[RET2:.*]]
----------------
vsk wrote:
> Please restructure these checks so they have a clear correspondence to a test function. The typical way to write this is:
> 
> ```
> ; CHECK-LABEL: some_test1
> ; CHECK: ...
> define void @some_test1
> 
> ; CHECK-LABEL: some_test2
> ; CHECK: ...
> define void @some_test2
> ```
> etc.
I was a bit lazy, sure, thanks!


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

https://reviews.llvm.org/D81939