[PATCH] D61198: [IRBuilder][DebugInfo] Don't pick DebugLocs for new instructions from debug intrinsics

Wed May 8 06:55:20 PDT 2019

jmorse abandoned this revision.
jmorse added a comment.

For some reason I hadn't clocked that the IRBuilder deals with blocks in a state of partial construction/correctness (duh), and had assumed there would always be a terminator in each block, whoops.

In D61198#1480767 <https://reviews.llvm.org/D61198#1480767>, @vsk wrote:

> There's a tension here between finding a narrow solution to the invalid-location bug and having SetInsertPoint() do something principled with debug locations. See D39982 <https://reviews.llvm.org/D39982> for a more detailed explanation of this.

Ouch, I have now been enlightened,

> I think it'd make sense to opt for a narrow solution here, but am not sure building on top of SetInsertPoint() is the way to go. What do folks think about this (possibly naive) alternative: if locations on debug intrinsics aren't meaningful, can we get rid of them (or use line 0 locations which just preserve scope info)? This would prevent SetInsertPoint from propagating misleading debug locations.

That sounds like a plan -- this patch was supposed to just reduce the circumstances where D59272 <https://reviews.llvm.org/D59272> was necessary, but as it seems to be a harder problem to solve than expected, we'll just have to rely on zero-line-nos-for-debug-intrinsics more. I've updated D59272 <https://reviews.llvm.org/D59272> accordingly.

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

https://reviews.llvm.org/D61198