[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.

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.

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...)


http://reviews.llvm.org/D17946