[PATCH] D33614: Cloning: Fix debug info cloning

[Gor Nishanov via Phabricator via llvm-commits]

GorNishanov added inline comments.


================
Comment at: lib/IR/DebugLoc.cpp:170
+
+    auto *NewVar = DILocalVariable::get(
+      Ctx,
----------------
GorNishanov wrote:
> GorNishanov wrote:
> > GorNishanov wrote:
> > > dblaikie wrote:
> > > > Is there a reason this can't be distinct anymore?
> > > > 
> > > > You mentioned in the description: "I added caching, but, it is not sufficient, since dbg.value may refer to various expressions representing the same user var, thus, switching form getDistinct to get is important."
> > > > 
> > > > But I don't follow quite what you're saying there - could you explain it in more detail/in another way?
> > > > 
> > > > What happens without changing this to 'get'?
> > > It can still creates duplicate DILocalVariables. I have it in the very large test case, I can see if I can reduce it to something manangable. Thank you for looking at this!
> > Hah. Cache by itself does not work since it is cleared for every basic block.
> > I am not sure what are the implications of moving it outside of CloneBasicBlock.
> > 
> > I would keep the cache where it is, and keep get as opposed to getDistinct to collapse duplicate LocalVariables to the same one.
> Good to go?
Alternative, since the cache scope is just one basic block, I can remove the caching from auto reparentVar altogether, so the fix in  `lib/IR/DebugLoc.cpp` will be simply:

  s/getDistinct/get 

in 

```
  auto reparentVar = [&](DILocalVariable *Var) {
      return DILocalVariable::getDistinct(
```


https://reviews.llvm.org/D33614