[PATCH] D41466: [Unroll][DebugInfo] Propagate loop body's debug location to epilog preheader

[Evgeny Stupachenko via Phabricator via llvm-commits]

evstupac added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:651-652
                                      DT, LI, PreserveLCSSA);
+    auto *TI = NewExit->getTerminator();
+    TI->setDebugLoc(Header->getTerminator()->getDebugLoc());
     // Split NewExit to insert epilog remainder loop.
----------------
zzheng wrote:
> evstupac wrote:
> > Am I right that this will set more DebugLoc for epilog case  than for prolog? If so, why?
> > If "TI" is supposed to be used only once I wouldn't create it at all:
> >   NewExit->getTerminator()->setDebugLoc(
> >       Header->getTerminator()->getDebugLoc());
> > If not - reuse it in the line 654 and give wider name (like NewExitTerminator).
> >  EpilogPreHeader = SplitBlock(NewExit, NewExitTerminator, DT, LI);
> > 
> NewExit gets its DebugLoc from LatchExit. This DebugLoc is not part of the original loop, it's after the original loop (say loc A).
> 
> The conditional branch deciding if epilog will be added to NewExit, which should be part of the original loop in lexical sense.
> 
> If a later pass search for debuginfo in RPOT order, it will first find loc A at the conditional branch rather than the merge point of unrolled and epilog loop.
> 
> On the other hand, PrologPreHeader gets its debug info from original PreHeader. 
Thanks for detailed explanation. Now it is clear for me. I'd add this as a comment:
// NewExit gets its DebugLoc from LatchExit, which do not belongs to the Loop.
// Fix this by setting Loop DebugLoc to NewExit.


Repository:
  rL LLVM

https://reviews.llvm.org/D41466