[PATCH] D16169: [Utils] Fix incorrect dbg.declare store conversion

[Keno Fischer via llvm-commits]

loladiro added inline comments.

================
Comment at: test/Transforms/Util/store-first-op.ll:13
@@ +12,3 @@
+; is correct.
+; CHECK-NOT: @llvm.dbg.value(metadata %foo* %getU
+  call void @llvm.dbg.declare(metadata %foo* %getU, metadata !3, metadata !6), !dbg !7
----------------
aprantl wrote:
> loladiro wrote:
> > aprantl wrote:
> > > I would still add a positive check — this way we'll be notified when the testcase needs to be updated.
> > What kind of positive test are you imagining. In the corrected version, no dbg.* intrinsic is inserted.
> My thought was that if no dbg.value is inserted the dbg.declare should still be there but that isn't actually the case. In this testcase the alloca is dead because there is no store to it or load, so not having any intrinsics makes sense.
> 
> Hm. This brings up an interesting point: Shouldn't LowerDbgDeclare() replace the alloca in the dbg.declare instruction with an %undef or alternatively leave the dbg.declare in there if the dbg.declare was not lowered? Otherwise we're going to loose function arguments and local variables.
Yes, I think that would be reasonable (but not for this patch - will go ahead and commit).


Repository:
  rL LLVM

http://reviews.llvm.org/D16169