[PATCH] D61184: [Salvage] Change salvage debug info implementation to use new DW_OP_LLVM_convert where needed

Fri Dec 13 01:27:00 PST 2019

StephenTozer marked an inline comment as done.
StephenTozer added inline comments.

================
Comment at: llvm/test/Transforms/InstCombine/pr43893.ll:16
   %conv = sext i8 %dec to i32, !dbg !17
-  call void @llvm.dbg.value(metadata i32 %conv, metadata !18, metadata !DIExpression()), !dbg !19
+  %udiv = udiv i32 %conv, 4, !dbg !17
+  call void @llvm.dbg.value(metadata i32 %udiv, metadata !18, metadata !DIExpression()), !dbg !19
----------------
vsk wrote:
> I'm not sure I understand this change. Why is the dbg.value for %conv deleted?
The purpose of this test as stated in the comment at the top was to check that, when the trivially deletable %conv was deleted, metadata that depended on it was set to undef. Reading that comment it seemed that the important component of the test wasn't the existence of the dbg.value itself, but confirming that it would be set `undef` rather than being given an incorrect value. As of this patch however, %conv can be salvaged, which seems to defeat the point of the test. As %udiv cannot be salvaged, it seemed like an appropriate stand-in.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61184