
<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hi everybody,</div><div class=""><br class=""></div><div class="">I'm investigating some correctness issues with debug line table information in optimized code. I've noticed something problematic in IRBuilder. Setting the insertion point of an IRBuilder sets the builder’s current debug loc to the one attached to the insertion point. IMO this isn't always a correct or convenient thing to do, and it shouldn't be the default behavior.</div><div class=""><br class=""></div><div class=""># Correctness</div><div class=""><br class=""></div><div class="">The IRBuilder shouldn't assume that the debug loc at its insertion point applies to any instructions it emits. It doesn't have enough information to make that connection. This assumption leads to bugs like <a href="http://llvm.org/PR25630" class="">llvm.org/PR25630</a>. I.e it creates situations in which we silently push around incorrect DILocations without realizing it, because the debug locs in the IR look nice and contiguous.</div><div class=""><br class=""></div><div class="">To fix the issue, clients of IRBuilder should be required to either:</div><div class=""><br class=""></div><div class="">1. Explicitly set the correct debug location before inserting instructions, or</div><div class="">2. Opt-in to the current behavior of propagating the debug loc from the insertion point.</div><div class=""><br class=""></div><div class=""># Convenience</div><div class=""><br class=""></div><div class="">The current behavior of IRBuilder can be convenient when it's correct. But it can be *really* inconvenient when it's incorrect.</div><div class=""><br class=""></div><div class="">If a client sets an IRBuilder's debug loc correctly and makes a call that changes the builder's insertion point, all generated instructions will pick up a bogus debug loc from the IP. E.g this is what happens with SCEVExpander::expandCodeFor(). There's no way to fix this bug without changing IRBuilder's behavior.</div><br class=""># Solutions<div class=""><br class=""></div><div class="">1. Add a method called SetInsertPointAndDebugLoc() which behaves identically to the current SetInsertPoint(). Change SetInsertPoint() so that it doesn't change the current debug loc.</div><div class="">2. Add a boolean argument to SetInsertPoint() called UpdateDbgLoc which defaults to false. Only change the current debug loc if UpdateDbgLoc is true.</div><div class=""><br class=""></div><div class="">I have a mild preference for idea #1 because it's easier to read, IMO.</div><div class=""><br class=""></div><div class="">Whichever solution we pick, my plan is to first migrate all in-tree users of SetInsertPoint() to the explicit API, and to then gradually fix passes which rely on the current incorrect behavior.</div><div class=""><br class=""></div><div class="">I'd appreciate your thoughts on this!</div><div class=""><br class=""></div><div class="">thanks,</div><div class="">vedant</div></body></html>