r246991 - When building the alloca for a local variable, set its name

[Daniel Dunbar via cfe-commits]

I agree with Chandler the default should be no names for -Asserts builds.

What I wanted originally was that the stripping of names be a runtime
choice (since the performance overhead of it was minimal when I measured
it), so that when we need to debug with a production compiler, it would
still be possible to get the names out. Sometimes that is invaluable when a
bug only handily reproduces with a production compiler you have on hand
(e.g., you might be at a users desktop) and it would help to see the names.

 - Daniel


On Tue, Sep 8, 2015 at 11:06 AM, David Blaikie via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

>
>
> On Tue, Sep 8, 2015 at 10:52 AM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
>> None of my performance concerns were relevant to this change FWIW.
>>
>
> Good to know - if we wanted to go down this path I figure we could just
> provide overloads - StringRef and Twine, the StringRef one could always use
> the name, even in -Asserts and the Twine one would not.
>
> But yeah, your comments below seem worth considering before we figure out
> about that path.
>
>
>> I think the reason that this got "fixed" was because people had a
>> tendancy to *rely* on the name downstream when we made it always present.
>> =/ Personally, I like having *no* names in a no-asserts build because it
>> ensures that absolutely no one is relying on them -- we have debug
>> information for that.
>>
>> I wonder if the right way to help the debugging scenario (which is very
>> real and painful) is to expose a flag for this, or to expose a way to take
>> an IR file with debug info and synthesize nice names for as much as we can
>> using the debug info?
>>
>> On Tue, Sep 8, 2015 at 10:47 AM David Blaikie via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> On Tue, Sep 8, 2015 at 10:26 AM, John McCall <rjmccall at apple.com> wrote:
>>>
>>>> On Sep 8, 2015, at 8:25 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>>> On Tue, Sep 8, 2015 at 2:18 AM, John McCall via cfe-commits <
>>>> cfe-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: rjmccall
>>>>> Date: Tue Sep  8 04:18:30 2015
>>>>> New Revision: 246991
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=246991&view=rev
>>>>> Log:
>>>>> When building the alloca for a local variable, set its name
>>>>> separately from building the instruction so that it's
>>>>> preserved even in -Asserts builds.
>>>>>
>>>>
>>>> Why do we want to preserve this name in particular in -Asserts builds?
>>>>
>>>>
>>>> It’s a fairly small runtime cost — no twine appending, only present on
>>>> a tiny fraction of instructions, unlikely to collide within a single
>>>> function — that makes reading the IR dramatically easier.  That’s still
>>>> useful to be able to do with a production compiler, both for us and for
>>>> sophisticated users.
>>>>
>>>> IIRC, Daniel did the perf work on IR names way back when; he might have
>>>> other thoughts.
>>>>
>>>
>>> *nod* I'd be curious to see/hear/better understand the tradeoffs. I know
>>> I've talked to Chandler (CC'd) about it before when he's expressed a desire
>>> to want to more fully remove names from non-asserts builds (specifically:
>>> optimizations still create named values even in -Asserts builds, I think?
>>> or maybe there was some other - oh, right, as you were saying, the Twine
>>> building cost, even when the name isn't used some names are complex to
>>> create, etc)
>>>
>>>
>>>>
>>>> John.
>>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150908/0e2de974/attachment-0001.html>