Remove compile time PreserveName switch based on NDEBUG

[mehdi_amini]

Summary

Following r263086, we are now relying on a flag on the Context to
discard Value names in release builds.

[chandlerc]

FYI, I'd lik eto hold off on submitting here until we're on the same page with Philip on the original LLVM review (D17946), but some comments on how this is working are below...

lib/CodeGen/CodeGenAction.cpp

660–663

I'm surprised this isn't a driver-side NDEBUG default flip? Can this still be overridden with a flag?

[mehdi_amini]

lib/CodeGen/CodeGenAction.cpp

660–663

It's gonna be next patch: plumbing a cc1 flag somehow. Right now this is minimum patch to maintain existing behavior in clang while adapting to the IRBuilder API in LLVM.

[chandlerc]

lib/CodeGen/CodeGenAction.cpp

660–663

Ah, just add a FIXME comment to make it clear that this is a transitional thing and not planned to be the end state?

[mehdi_amini]

Add a FIXME to change the compile time switch to a runtime command line flag

[chandlerc]

LGTM

[mehdi_amini]

r263257

lib/CodeGen/CGBuilder.h

 namespace clang {
 namespace CodeGen {
 
 class CodeGenFunction;
 
 /// \brief This is an IRBuilder insertion helper that forwards to
 /// CodeGenFunction::InsertHelper, which adds necessary metadata to
 /// instructions.
-template <bool PreserveNames>
-class CGBuilderInserter
-    : protected llvm::IRBuilderDefaultInserter<PreserveNames> {
+class CGBuilderInserter : protected llvm::IRBuilderDefaultInserter {
 public:
   CGBuilderInserter() = default;
   explicit CGBuilderInserter(CodeGenFunction *CGF) : CGF(CGF) {}
 
 protected:
   /// \brief This forwards to CodeGenFunction::InsertHelper.
   void InsertHelper(llvm::Instruction *I, const llvm::Twine &Name,
                     llvm::BasicBlock *BB,
                     llvm::BasicBlock::iterator InsertPt) const;
 private:
   CodeGenFunction *CGF = nullptr;
 };
 
-// Don't preserve names on values in an optimized build.
-#ifdef NDEBUG
-#define PreserveNames false
-#else
-#define PreserveNames true
-#endif
-
-typedef CGBuilderInserter<PreserveNames> CGBuilderInserterTy;
+typedef CGBuilderInserter CGBuilderInserterTy;
 
-typedef llvm::IRBuilder<PreserveNames, llvm::ConstantFolder,
-                        CGBuilderInserterTy> CGBuilderBaseTy;
+typedef llvm::IRBuilder<llvm::ConstantFolder, CGBuilderInserterTy>
+    CGBuilderBaseTy;
 
 class CGBuilderTy : public CGBuilderBaseTy {
   /// Storing a reference to the type cache here makes it a lot easier
   /// to build natural-feeling, target-specific IR.
   const CodeGenTypeCache &TypeCache;
 public:
   CGBuilderTy(const CodeGenTypeCache &TypeCache, llvm::LLVMContext &C)
     : CGBuilderBaseTy(C), TypeCache(TypeCache) {}
public:
   using CGBuilderBaseTy::CreateMemSet;
   llvm::CallInst *CreateMemSet(Address Dest, llvm::Value *Value,
                                llvm::Value *Size, bool IsVolatile = false) {
     return CreateMemSet(Dest.getPointer(), Value, Size,
                         Dest.getAlignment().getQuantity(), IsVolatile);
   }
 };
 
-#undef PreserveNames
-
 }  // end namespace CodeGen
 }  // end namespace clang
 
 #endif

lib/CodeGen/CGCall.cpp

if (CS.doesNotReturn()) {
     // points.
     EnsureInsertPoint();
 
     // Return a reasonable RValue.
     return GetUndefRValue(RetTy);
   }
 
   llvm::Instruction *CI = CS.getInstruction();
-  if (Builder.isNamePreserving() && !CI->getType()->isVoidTy())
+  if (!CI->getType()->isVoidTy())
     CI->setName("call");
 
   // Emit any writebacks immediately.  Arguably this should happen
   // after any return-value munging.
   if (CallArgs.hasWritebacks())
     emitWritebacks(*this, CallArgs);
 
   // The stack cleanup for inalloca arguments has to run out of the normal

lib/CodeGen/CGExpr.cpp

Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
   Alloca->setAlignment(Align.getQuantity());
   return Address(Alloca, Align);
 }
 
 /// CreateTempAlloca - This creates a alloca and inserts it into the entry
 /// block.
 llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
                                                     const Twine &Name) {
-  if (!Builder.isNamePreserving())
-    return new llvm::AllocaInst(Ty, nullptr, "", AllocaInsertPt);
   return new llvm::AllocaInst(Ty, nullptr, Name, AllocaInsertPt);
 }
 
 /// CreateDefaultAlignTempAlloca - This creates an alloca with the
 /// default alignment of the corresponding LLVM type, which is *not*
 /// guaranteed to be related in any way to the expected alignment of
 /// an AST type that might have been lowered to Ty.
 Address CodeGenFunction::CreateDefaultAlignTempAlloca(llvm::Type *Ty,

lib/CodeGen/CodeGenAction.cpp

void BackendConsumer::DiagnosticHandlerImpl(const DiagnosticInfo &DI) {
   // Report the backend message using the usual diagnostic mechanism.
   FullSourceLoc Loc;
   Diags.Report(Loc, DiagID).AddString(MsgStorage);
 }
 #undef ComputeDiagID
 
 CodeGenAction::CodeGenAction(unsigned _Act, LLVMContext *_VMContext)
     : Act(_Act), VMContext(_VMContext ? _VMContext : new LLVMContext),
-      OwnsVMContext(!_VMContext) {}
+      OwnsVMContext(!_VMContext) {
+#ifdef NDEBUG
+  // FIXME: change this to be controlled by a cc1 flag that the driver passes,
+  // on the model of --disable-free
+  _VMContext.setDiscardValueNames(true);
+#endif
+}
 
 CodeGenAction::~CodeGenAction() {
   TheModule.reset();
   if (OwnsVMContext)
     delete VMContext;
 }
 
 bool CodeGenAction::hasIRSupport() const { return true; }

lib/CodeGen/CodeGenFunction.cpp

if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
         Fn->addFnAttr(llvm::Attribute::NoRecurse);
   
   llvm::BasicBlock *EntryBB = createBasicBlock("entry", CurFn);
 
   // Create a marker to make it easy to insert allocas into the entryblock
   // later.  Don't create this with the builder, because we don't want it
   // folded.
   llvm::Value *Undef = llvm::UndefValue::get(Int32Ty);
-  AllocaInsertPt = new llvm::BitCastInst(Undef, Int32Ty, "", EntryBB);
-  if (Builder.isNamePreserving())
-    AllocaInsertPt->setName("allocapt");
+  AllocaInsertPt = new llvm::BitCastInst(Undef, Int32Ty, "allocapt", EntryBB);
 
   ReturnBlock = getJumpDestInCurrentScope("return");
 
   Builder.SetInsertPoint(EntryBB);
 
   // Emit subprogram debug descriptor.
   if (CGDebugInfo *DI = getDebugInfo()) {
     SmallVector<QualType, 16> ArgTypes;
void CodeGenFunction::InsertHelper(llvm::Instruction *I,
                                    const llvm::Twine &Name,
                                    llvm::BasicBlock *BB,
                                    llvm::BasicBlock::iterator InsertPt) const {
   LoopStack.InsertHelper(I);
   if (IsSanitizerScope)
     CGM.getSanitizerMetadata()->disableSanitizerForInstruction(I);
 }
 
-template <bool PreserveNames>
-void CGBuilderInserter<PreserveNames>::InsertHelper(
+void CGBuilderInserter::InsertHelper(
     llvm::Instruction *I, const llvm::Twine &Name, llvm::BasicBlock *BB,
     llvm::BasicBlock::iterator InsertPt) const {
-  llvm::IRBuilderDefaultInserter<PreserveNames>::InsertHelper(I, Name, BB,
-                                                              InsertPt);
+  llvm::IRBuilderDefaultInserter::InsertHelper(I, Name, BB, InsertPt);
   if (CGF)
     CGF->InsertHelper(I, Name, BB, InsertPt);
 }
 
-#ifdef NDEBUG
-#define PreserveNames false
-#else
-#define PreserveNames true
-#endif
-template void CGBuilderInserter<PreserveNames>::InsertHelper(
-    llvm::Instruction *I, const llvm::Twine &Name, llvm::BasicBlock *BB,
-    llvm::BasicBlock::iterator InsertPt) const;
-#undef PreserveNames
-
 static bool hasRequiredFeatures(const SmallVectorImpl<StringRef> &ReqFeatures,
                                 CodeGenModule &CGM, const FunctionDecl *FD,
                                 std::string &FirstMissing) {
   // If there aren't any required features listed then go ahead and return.
   if (ReqFeatures.empty())
     return false;
 
   // Now build up the set of caller features and verify that all the required