<div dir="ltr">Yeah. That is what I ended up doing in the latest revision. <a href="https://reviews.llvm.org/D33614">https://reviews.llvm.org/D33614</a></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 26, 2017 at 9:41 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">distinct nodes are a bit more expensive (due to the generalized deduplication) so probably better to not rely on them. Alternatively, if you are relying on tehm - why not rely on them solely, rather than usuing the cache as well? Seems weird to have a hybrid approach.</div><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Fri, May 26, 2017 at 9:24 PM Gor Nishanov via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">GorNishanov added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/IR/DebugLoc.cpp:170<br>
+<br>
+    auto *NewVar = DILocalVariable::get(<br>
+      Ctx,<br>
----------------<br>
GorNishanov wrote:<br>
> dblaikie wrote:<br>
> > Is there a reason this can't be distinct anymore?<br>
> ><br>
> > 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."<br>
> ><br>
> > But I don't follow quite what you're saying there - could you explain it in more detail/in another way?<br>
> ><br>
> > What happens without changing this to 'get'?<br>
> 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!<br>
Hah. Cache by itself does not work since it is cleared for every basic block.<br>
I am not sure what are the implications of moving it outside of CloneBasicBlock.<br>
<br>
I would keep the cache where it is, and keep get as opposed to getDistinct to collapse duplicate LocalVariables to the same one.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D33614" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D33614</a><br>
<br>
<br>
<br>
</blockquote></div>
</div></div></blockquote></div><br></div>