[PATCH] D59272: [DebugInfo] Select debug intrinsic line-numbers more carefully when promoting dbg.declare

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked 4 inline comments as done.
jmorse added inline comments.


================
Comment at: lib/Transforms/Utils/Local.cpp:1311
 
+  DebugLoc NewLoc = getDebugValueLoc(DII, SI);
+
----------------
bjope wrote:
> Current solution is fine with me (as it seems to be one step in the right direction).
> 
> Alternatives solutions could be:
> * Let DIBuilder::insertDbgValueIntrinsic strip the line number? (that would also cover the creation of dbg.value in LoopUtils, Debugify and LLVMDIBuilder*)
> * Strip line/col number already when creating dbg.declare? (or do we need it in dbg.declare)
> 
> Some additional thing in this patch could be (if this is a new rule):
> * Also fix calls to DIBuilder::insertDbgValueIntrinsic in LoopUtils, Debugify and LLVMDIBuilder* the same way.
> * Assert that line/col is zero in DIBuilder::insertDbgValueIntrinsic.
> * Add checks in Verifier and MachineVerifier that line/col is zero for dbg.value/DBG_VALUE.
> 
Bjorn wrote:
> Some additional thing in this patch could be (if this is a new rule):
> 
> Also fix calls to DIBuilder::insertDbgValueIntrinsic in LoopUtils, Debugify and LLVMDIBuilder* the same way.
> Assert that line/col is zero in DIBuilder::insertDbgValueIntrinsic.
> Add checks in Verifier and MachineVerifier that line/col is zero for dbg.value/DBG_VALUE.

Sounds good -- I've dropped those details in this bug report [0] so that they don't get lost, the Verifier checks in particular should be good for detecting error states.

[0] https://bugs.llvm.org/show_bug.cgi?id=41827


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59272