[PATCH] D149335: [DebugInfo][InstCombine] Fix missing source and variable locations after foldOpIntoPhi

[Felipe de Azevedo Piovezan via Phabricator via llvm-commits]

fdeazeve accepted this revision as: fdeazeve.
fdeazeve added a comment.
This revision is now accepted and ready to land.

Nice catch!



================
Comment at: llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll:7
+
+;; CHECK: %p.0 = phi i8 [ 1, %if.then ], [ 0, %entry ], !dbg ![[#]]
+;; CHECK: call void @llvm.dbg.value(metadata i8 %p.0, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)
----------------
fdeazeve wrote:
> nit: it's probably better to avoid hardcoding the virtual register names
Was the double `;` intentional here?


================
Comment at: llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll:7-8
+
+;; CHECK: %p.0 = phi i8 [ 1, %if.then ], [ 0, %entry ], !dbg ![[#]]
+;; CHECK: call void @llvm.dbg.value(metadata i8 %p.0, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)
+
----------------
nit: it's probably better to avoid hardcoding the virtual register names


================
Comment at: llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll:30
+declare float @ff(float)
+attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+
----------------
I think this is not used anywhere


================
Comment at: llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll:48
+!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!13 = !DILocation(line: 0, scope: !5)
----------------
Not sure if worth it, but we could give this an unusual line number and check that the converted phi has a location with that same number.


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

https://reviews.llvm.org/D149335