[PATCH] D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics

Fri Feb 22 05:15:16 PST 2019

bjope added a comment.

`placeDbgValues` has been bugging me (and probably others) for some time. So I'm happy with this patch.

Haven't done any recent tests myself, but hopefully we have solved most problems that has blocked this change by now. I know that @jmorse has done a splendid job in hunting down and fixing such problems.
As this patch is a compromising solution, we still sink the dbg.value when having use-before-def, the patch seem reasonable to me at this point in time. It might unveil a few not yet detected problems, but those would probably be hard to find unless we do this.
One idea is to have a command-line option to make it easier to revert to the old behavior (if we find some bigger problems), but I do not think it is worth the trouble.

So this LGTM now. But it looks like @probinson had some questions, so I'm not setting "ready to land" right now.

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

https://reviews.llvm.org/D58453