[PATCH] D48968: [DebugInfo][LoopVectorize] Preserve DL in induction PHI and Add

[Anastasis via Phabricator via llvm-commits]

gramanas marked an inline comment as done.
gramanas added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1824
                                     &*LoopVectorBody->getFirstInsertionPt());
+  VecInd->setDebugLoc(EntryVal->getDebugLoc());
   Instruction *LastInduction = VecInd;
----------------
vsk wrote:
> Why is the debug location of the entry value the correct location to use for an induction variable phi? I'd expect a merged location to be applied to the phi, based on its incoming values.
I figured that since this Phi is basically the for loop we are optimizing (in the test case below)
`%vec.ind = phi <4 x i8> [ undef, %vector.ph ], [ %vec.ind.next, %vector.body ]` 
it should get the same DL as the add instruction later. 

Also the entry val as mentioned in `InnerLoopVectorizer::widenIntOrFpInduction` (that calls the function I edited) is 
`// The value from the original loop to which we are mapping the new induction variable.` 
that I believe holds the correct DL.

OTOH, I could merge this DL with the vector preHeader BB's terminating instruction that results in this phi. Would this be a better approach?
Basically I could add `setDebugLoc(DILocation::getMergedLocation(EntryVal->getDL(), Br->getDL())` after the Phi's incoming values are added, at the end of the function. 


================
Comment at: test/Transforms/LoopVectorize/i8-induction.ll:15
+;
+; DEBUGLOC:         ![[DbgLoc]] = !DILocation(line: 5
 scalar.ph:
----------------
vsk wrote:
> vsk wrote:
> > A reader might not expect to see a DILocation here. Please move this to the end of the file. That's where the metadata would appear in an IR dump.
> Please add a comment explaining why the number '5' is here.
I wanted to make sure that it's not an artificial DL. I made it explicit that it's not `line: 0` now.


Repository:
  rL LLVM

https://reviews.llvm.org/D48968