[CodeGen] Don't emit the same global block multiple times.

[george.burgess.iv]

Summary

D14274 makes our constexpr evaluator more aggressive with some variables marked const. This changes how we behave on code like the following:

void foo() {
  void (^const block_A)(void) = ^{ return; };
  get_kernel_work_group_size(block_A);
  get_kernel_work_group_size(block_A);
}

The constexpr evaluator will now give us ^{ return; } three times (one for each use of block_A) instead of once (in block_A's assignment). CodeGen emits a block every time it gets handed a BlockExpr, so we end up emitting the code for ^{ return; } three times in total. We can fix this by tracking which global BlockExprs we've already generated code for.

This seems to not happen for local BlockExprs, since the constexpr fails for BlockExprs with captures (see PointerExprEvaluator::VisitBlockExpr in lib/AST/ExprConstant.cpp). That said, I'm happy to add this uniquing code for BlockExprs with captures if anyone wants me to.

[rjmccall]

This looks great. One minor tweak and then it's ready.

lib/CodeGen/CGBlocks.cpp

1057

We conventionally name these things "set" rather than "put".

[george.burgess.iv]

Thanks for the review!

I'll submit this with D14274. :)

[george.burgess.iv]

Committed in rL290149. Thanks again!

lib/CodeGen/CGBlocks.cpp

void CodeGenFunction::destroyBlockInfos(CGBlockInfo *head) {
   } while (head != nullptr);
 }
 
 /// Emit a block literal expression in the current function.
 llvm::Value *CodeGenFunction::EmitBlockLiteral(const BlockExpr *blockExpr) {
   // If the block has no captures, we won't have a pre-computed
   // layout for it.
   if (!blockExpr->getBlockDecl()->hasCaptures()) {
+    if (llvm::Constant *Block = CGM.getAddrOfGlobalBlockIfEmitted(blockExpr))
+      return Block;
     CGBlockInfo blockInfo(blockExpr->getBlockDecl(), CurFn->getName());
     computeBlockInfo(CGM, this, blockInfo);
     blockInfo.BlockExpression = blockExpr;
     return EmitBlockLiteral(blockInfo);
   }
 
   // Find the block info for this block and take ownership of it.
   std::unique_ptr<CGBlockInfo> blockInfo;
Address CodeGenFunction::GetAddrOfBlockDecl(const VarDecl *variable,
   }
 
   if (auto refType = capture.fieldType()->getAs<ReferenceType>())
     addr = EmitLoadOfReference(addr, refType);
 
   return addr;
 }
 
+void CodeGenModule::setAddrOfGlobalBlock(const BlockExpr *BE,
+                                         llvm::Constant *Addr) {
+  bool Ok = EmittedGlobalBlocks.insert(std::make_pair(BE, Addr)).second;
+  (void)Ok;
+  assert(Ok && "Trying to replace an already-existing global block!");
+}
+
 llvm::Constant *
 CodeGenModule::GetAddrOfGlobalBlock(const BlockExpr *BE,
                                     StringRef Name) {
+  if (llvm::Constant *Block = getAddrOfGlobalBlockIfEmitted(BE))
+    return Block;
+
   CGBlockInfo blockInfo(BE->getBlockDecl(), Name);
   blockInfo.BlockExpression = BE;
 
   // Compute information about the layout, etc., of this block.
   computeBlockInfo(*this, nullptr, blockInfo);
 
   // Using that metadata, generate the actual block function.
   llvm::Constant *blockFn;
   {
     CodeGenFunction::DeclMapTy LocalDeclMap;
     blockFn = CodeGenFunction(*this).GenerateBlockFunction(GlobalDecl(),
                                                            blockInfo,
                                                            LocalDeclMap,
                                                            false);
   }
   blockFn = llvm::ConstantExpr::getBitCast(blockFn, VoidPtrTy);
 
   return buildGlobalBlock(*this, blockInfo, blockFn);
 }
 
 static llvm::Constant *buildGlobalBlock(CodeGenModule &CGM,
                                         const CGBlockInfo &blockInfo,
                                         llvm::Constant *blockFn) {
   assert(blockInfo.CanBeGlobal);
+  // Callers should detect this case on their own: calling this function
+  // generally requires computing layout information, which is a waste of time
+  // if we've already emitted this block.
+  assert(!CGM.getAddrOfGlobalBlockIfEmitted(blockInfo.BlockExpression) &&
+         "Refusing to re-emit a global block.");
 
   // Generate the constants for the block literal initializer.
   llvm::Constant *fields[BlockHeaderSize];
 
   // isa
   fields[0] = CGM.getNSConcreteGlobalBlock();
 
   // __flags
   BlockFlags flags = BLOCK_IS_GLOBAL | BLOCK_HAS_SIGNATURE;
   if (blockInfo.UsesStret) flags |= BLOCK_USE_STRET;
-                                      
   fields[1] = llvm::ConstantInt::get(CGM.IntTy, flags.getBitMask());
 
   // Reserved
   fields[2] = llvm::Constant::getNullValue(CGM.IntTy);
 
   // Function
   fields[3] = blockFn;
 
   // Descriptor
   fields[4] = buildBlockDescriptor(CGM, blockInfo);
 
   llvm::Constant *init = llvm::ConstantStruct::getAnon(fields);
 
   llvm::GlobalVariable *literal =
     new llvm::GlobalVariable(CGM.getModule(),
                              init->getType(),
                              /*constant*/ true,
                              llvm::GlobalVariable::InternalLinkage,
                              init,
                              "__block_literal_global");
   literal->setAlignment(blockInfo.BlockAlign.getQuantity());
 
   // Return a constant of the appropriately-casted type.
-  llvm::Type *requiredType =
+  llvm::Type *RequiredType =
     CGM.getTypes().ConvertType(blockInfo.getBlockExpr()->getType());
-  return llvm::ConstantExpr::getBitCast(literal, requiredType);
+  llvm::Constant *Result =
+      llvm::ConstantExpr::getBitCast(literal, RequiredType);
+  CGM.setAddrOfGlobalBlock(blockInfo.BlockExpression, Result);
+  return Result;
 }
 
 void CodeGenFunction::setBlockContextParameter(const ImplicitParamDecl *D,
                                                unsigned argNum,
                                                llvm::Value *arg) {
   assert(BlockInfo && "not emitting prologue of block invocation function?!");
 
   llvm::Value *localAddr = nullptr;

lib/CodeGen/CodeGenFunction.h

void generateObjCSetterBody(const ObjCImplementationDecl *classImpl,
                               const ObjCPropertyImplDecl *propImpl,
                               llvm::Constant *AtomicHelperFn);
 
   //===--------------------------------------------------------------------===//
   //                                  Block Bits
   //===--------------------------------------------------------------------===//
 
   llvm::Value *EmitBlockLiteral(const BlockExpr *);
-  llvm::Value *EmitBlockLiteral(const CGBlockInfo &Info);
   static void destroyBlockInfos(CGBlockInfo *info);
 
   llvm::Function *GenerateBlockFunction(GlobalDecl GD,
                                         const CGBlockInfo &Info,
                                         const DeclMapTy &ldm,
                                         bool IsLambdaConversionToBlock);
 
   llvm::Constant *GenerateCopyHelperFunction(const CGBlockInfo &blockInfo);
void EmitOMPInnerLoop(
       const llvm::function_ref<void(CodeGenFunction &)> &PostIncGen);
 
   JumpDest getOMPCancelDestination(OpenMPDirectiveKind Kind);
   /// Emit initial code for loop counters of loop-based directives.
   void EmitOMPPrivateLoopCounters(const OMPLoopDirective &S,
                                   OMPPrivateScope &LoopScope);
 
 private:
+  /// Helpers for blocks
+  llvm::Value *EmitBlockLiteral(const CGBlockInfo &Info);
+
   /// Helpers for the OpenMP loop directives.
   void EmitOMPLoopBody(const OMPLoopDirective &D, JumpDest LoopExit);
   void EmitOMPSimdInit(const OMPLoopDirective &D, bool IsMonotonic = false);
   void EmitOMPSimdFinal(
       const OMPLoopDirective &D,
       const llvm::function_ref<llvm::Value *(CodeGenFunction &)> &CondGen);
   /// \brief Emit code for the worksharing loop-based directive.
   /// \return true, if this construct has any lastprivate clause, false -

lib/CodeGen/CodeGenModule.h

private:
 
   void createOpenCLRuntime();
   void createOpenMPRuntime();
   void createCUDARuntime();
 
   bool isTriviallyRecursive(const FunctionDecl *F);
   bool shouldEmitFunction(GlobalDecl GD);
 
+  /// Map of the global blocks we've emitted, so that we don't have to re-emit
+  /// them if the constexpr evaluator gets aggressive.
+  llvm::DenseMap<const BlockExpr *, llvm::Constant *> EmittedGlobalBlocks;
+
   /// @name Cache for Blocks Runtime Globals
   /// @{
 
   llvm::Constant *NSConcreteGlobalBlock = nullptr;
   llvm::Constant *NSConcreteStackBlock = nullptr;
 
   llvm::Constant *BlockObjectAssign = nullptr;
   llvm::Constant *BlockObjectDispose = nullptr;
public:
   /// Fetches the type of a generic block descriptor.
   llvm::Type *getBlockDescriptorType();
 
   /// The type of a generic block literal.
   llvm::Type *getGenericBlockLiteralType();
 
   /// Gets the address of a block which requires no captures.
   llvm::Constant *GetAddrOfGlobalBlock(const BlockExpr *BE, StringRef Name);
-  
+  /// Returns the address of a block which requires no caputres, or null if
+  /// we've yet to emit the block for BE.
+  llvm::Constant *getAddrOfGlobalBlockIfEmitted(const BlockExpr *BE) {
+    return EmittedGlobalBlocks.lookup(BE);
+  }
+
+  /// Notes that BE's global block is available via Addr. Asserts that BE
+  /// isn't already emitted.
+  void setAddrOfGlobalBlock(const BlockExpr *BE, llvm::Constant *Addr);
+  
   /// Return a pointer to a constant CFString object for the given string.
   ConstantAddress GetAddrOfConstantCFString(const StringLiteral *Literal);
 
   /// Return a pointer to a constant NSString object for the given string. Or a
   /// user defined String object as defined via
   /// -fconstant-string-class=class_name option.
   ConstantAddress GetAddrOfConstantString(const StringLiteral *Literal);
 

test/CodeGenCXX/block-in-ctor-dtor.cpp

 
 
 // CHECK-LABEL: define internal void @___ZN4ZoneC2Ev_block_invoke
 // CHECK-LABEL: define internal void @___ZN4ZoneC2Ev_block_invoke_
 // CHECK-LABEL: define internal void @___ZN4ZoneD2Ev_block_invoke
 // CHECK-LABEL: define internal void @___ZN4ZoneD2Ev_block_invoke_
 // CHECK-LABEL: define internal void @___ZN1XC2Ev_block_invoke
 // CHECK-LABEL: define internal void @___ZN1XC2Ev_block_invoke_
-// CHECK-LABEL: define internal void @___ZN1XC1Ev_block_invoke
-// CHECK-LABEL: define internal void @___ZN1XC1Ev_block_invoke_
 // CHECK-LABEL: define internal void @___ZN1XD2Ev_block_invoke
 // CHECK-LABEL: define internal void @___ZN1XD2Ev_block_invoke_

test/CodeGenOpenCL/cl20-device-side-enqueue.cl

 // RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -O0 -emit-llvm -o - | FileCheck %s
 
 typedef void (^bl_t)(local void *);
 
+// N.B. The check here only exists to set BL_GLOBAL
+// CHECK: @block_G = {{.*}}bitcast ([[BL_GLOBAL:[^@]+@__block_literal_global(\.[0-9]+)?]]
 const bl_t block_G = (bl_t) ^ (local void *a) {};
 
 kernel void device_side_enqueue(global int *a, global int *b, int i) {
   // CHECK: %default_queue = alloca %opencl.queue_t*
   queue_t default_queue;
   // CHECK: %flags = alloca i32
   unsigned flags = 0;
   // CHECK: %ndrange = alloca %opencl.ndrange_t*
kernel void device_side_enqueue(global int *a, global int *b, int i) {
   // CHECK: [[SIZE:%[0-9]+]] = zext i8 {{%[0-9]+}} to i32
   // CHECK: call i32 (%opencl.queue_t*, i32, %opencl.ndrange_t*, i32, %opencl.clk_event_t**, %opencl.clk_event_t**, i8*, i32, ...) @__enqueue_kernel_events_vaargs(%opencl.queue_t* [[DEF_Q]], i32 [[FLAGS]], %opencl.ndrange_t* [[NDR]], i32 2, %opencl.clk_event_t** [[AD]], %opencl.clk_event_t** %clk_event, i8* bitcast ({ i8**, i32, i32, i8*, %struct.__block_descriptor addrspace(3)* }* @__block_literal_global{{(.[0-9]+)?}} to i8*), i32 1, i32 [[SIZE]])
   enqueue_kernel(default_queue, flags, ndrange, 2, event_wait_list2, &clk_event,
                  ^(local void *p) {
                    return;
                  },
                  c);
 
+  // The full type of these expressions are long (and repeated elsewhere), so we
+  // capture it as part of the regex for convenience and clarity.
+  // CHECK: store void ()* bitcast ([[BL_A:[^@]+@__block_literal_global.[0-9]+]] to void ()*), void ()** %block_A
   void (^const block_A)(void) = ^{
     return;
   };
+  // CHECK: store void (i8 addrspace(2)*)* bitcast ([[BL_B:[^@]+@__block_literal_global.[0-9]+]] to void (i8 addrspace(2)*)*), void (i8 addrspace(2)*)** %block_B
   void (^const block_B)(local void *) = ^(local void *a) {
     return;
   };
 
-  // CHECK: [[BL:%[0-9]+]] = load void ()*, void ()** %block_A
-  // CHECK: [[BL_I8:%[0-9]+]] = bitcast void ()* [[BL]] to i8*
-  // CHECK: call i32 @__get_kernel_work_group_size_impl(i8* [[BL_I8]])
+  // CHECK: call i32 @__get_kernel_work_group_size_impl(i8* bitcast ([[BL_A]] to i8*))
   unsigned size = get_kernel_work_group_size(block_A);
-  // CHECK: [[BL:%[0-9]+]] = load void (i8 addrspace(2)*)*, void (i8 addrspace(2)*)** %block_B
-  // CHECK: [[BL_I8:%[0-9]+]] = bitcast void (i8 addrspace(2)*)* [[BL]] to i8*
-  // CHECK: call i32 @__get_kernel_work_group_size_impl(i8* [[BL_I8]])
+  // CHECK: call i32 @__get_kernel_work_group_size_impl(i8* bitcast ([[BL_B]] to i8*))
   size = get_kernel_work_group_size(block_B);
-  // CHECK: [[BL:%[0-9]+]] = load void ()*, void ()** %block_A
-  // CHECK: [[BL_I8:%[0-9]+]] = bitcast void ()* [[BL]] to i8*
-  // CHECK: call i32 @__get_kernel_preferred_work_group_multiple_impl(i8* [[BL_I8]])
+  // CHECK: call i32 @__get_kernel_preferred_work_group_multiple_impl(i8* bitcast ([[BL_A]] to i8*))
   size = get_kernel_preferred_work_group_size_multiple(block_A);
-  // CHECK: [[BL:%[0-9]+]] = load void (i8 addrspace(2)*)*, void (i8 addrspace(2)*)* addrspace(1)* @block_G
-  // CHECK: [[BL_I8:%[0-9]+]] = bitcast void (i8 addrspace(2)*)* [[BL]] to i8*
-  // CHECK: call i32 @__get_kernel_preferred_work_group_multiple_impl(i8* [[BL_I8]])
+  // CHECK: call i32 @__get_kernel_preferred_work_group_multiple_impl(i8* bitcast ([[BL_GLOBAL]] to i8*))
   size = get_kernel_preferred_work_group_size_multiple(block_G);
 }