[PATCH] D17946: Add a flag to the LLVMContext to disable name for Value other than GlobalValue

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

In http://reviews.llvm.org/D17946#372751, @reames wrote:

> In http://reviews.llvm.org/D17946#371699, @chandlerc wrote:
>
> > LGTM with the nits below fixed and with whatever name we finally settle on. (We can also tweak the name later if needed). But sync up with Justin before you land it to be sure he's OK.
> >
> > In http://reviews.llvm.org/D17946#371474, @reames wrote:
> >
> > > In http://reviews.llvm.org/D17946#371188, @joker.eph wrote:
> > >
> > > > Hey Philip,
> > > >
> > > > Thanks for chiming in.
> > > >  Can you articulate what you have in mind for "an invariant for the optimizer wherein we only introduce named instructions if the instruction being replaced was named. Same with basic blocks."?
> > > >  I don't visualize how it would be expressed in practice, unless you intend to audit all the passes and change all the places where prefix/suffix are introduced?
> > >
> > >
> > > Yes.  If we have code which unconditionally adds a suffix, it would become if (hasName() ? getName() + "suffix" : "").  This seems entirely reasonable.
> >
> >
> > I totally understand why you want the names -- I agree they're incredibly important for debugging.
> >
> > I'm surprised you don't find the approach Mehdi is pursuing here strictly superior though. This essentially lets us completely remove the NDEBUG disabling of names. That means you can always pass a flag to Clang and get names in the output for debugging.
> >
> > I find this specifically nicer than keying on whether the original value had a name because there are a number of passes that produce values without names. Even when they do that, I would very much like to have the name suffixes introduced by subsequent passes when I'm debugging.
>
>
> I hadn't thought about this aspect.  Good point.
>
> > Does that make sense?
>
> > 
>
> > (I'd rather not go converting all of LLVM and Clang to rely on this approach unless you're comfortable with it...)
>
>
> I don't have a strong opinion here.  I was pointing out what seemed like a natural approach to me, but I'm also more than comfortable deferring to you and others who have thought about this more.  Provided that the default is to have names even in release mode (default for LLVM, not necessarily clang), I'm comfortable with whatever direction you want to take this.


Awesome, and thanks for following up.

I think this will end up with essentially the best of all worlds in that at all places where we can have extra information we will, and when we don't need it, we can turn off the cost of preserving it.


http://reviews.llvm.org/D17946