[llvm] r237827 - Add bool to DebugLocDwarfExpression to control emitting comments.

[Pete Cooper]

> On May 20, 2015, at 3:43 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> 
>> On 2015 May 20, at 13:58, Pete Cooper <peter_cooper at apple.com> wrote:
>> 
>>> 
>>> On May 20, 2015, at 1:54 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> 
>>> 
>>> 
>>> On Wed, May 20, 2015 at 1:46 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> 
>>> 
>>> On Wed, May 20, 2015 at 1:38 PM, Pete Cooper <peter_cooper at apple.com> wrote:
>>> Looks like there wasn’t worth using anything in the original commit.  I’ll revert it shortly.
>>> 
>>> What do you think of this one?
>>> 
>>> Looks plausible, but looping in Duncan because I assume his r235229 (April 17) was the cause of this?
>>> 
>>> Is it possible we could avoid the extra stream/buffer entirely and just stream them out from collectVariableInfo? I assume this would fragment the debug info data, though (since we'd be switching sections back & forth) - I'm not sure if that's a bad/problematic thing... 
>>> 
>>> I guess that's the 3rd fixme? Perhaps worth considering together.
>>> 
>>> Also I'm a bit confused about this design, now that I look at it a bit more... 
>>> 
>>> Seems a bit awkward and the original commit doesn't mention the savings (or maybe it does) - mentions 10-15% of memory allocation coming from that function, not sure how much was avoided by this change. 
>> Sorry, should have mentioned numbers.  This is a drop in the ocean (10000 allocations generating 300KB of data), and pretty much entirely unrelated to Duncan’s recent email.  Same files, but not on the path he was looking at.
>> 
>> Seemed like an easy fix for the sake of 10000 allocations saved which is why I did it.  Had it been fewer I doubt i’d have tried TBH.
>>> 
>>> Rambling: This doesn't seem like a stream any more than the original data structure, it's the "array of structs to struct of arrays" transformation... I would imagine we could make that a bit of a nicer abstraction than what's here, but maybe I'm missing something. Seems rather awkward having to pull out the components (comments, etc) by lookup of Entry rather than getting a whole Entry. Having the iterator over the loc lists produce whole objects that reference the various vector contents would be nice (proxy object - doesn't add allocation overhead, but gives a unified view of the list in the same pivot as the code before this commit, etc)
>> Yeah, i’m open to improving the structure here.  Let me take a look at collectVariableInfo as you pointed out.  Perhaps there’s a much nicer solution if we start from there, or even just emit the comments directly.
>> 
>> Pete
> 
> IMO, any next steps should just emit this stuff directly to the
> output section, rather than holding onto them to emit later.
> It's not clear to me how hard that is, but it would be a nice
> cleanup.
Sounds good.  I’ll take a look and see what needs to be done for this.

BTW, the original commit here was reverted in r237849 with the new patch applied in r237851.

Cheers,
Pete
>