Simplify codegen and debug info generation for block context parameters.

[aprantl]

Summary

The exisiting code goes out of its way to put block parameters into an alloca at -O0 only, but then describes the function argument (instead of the alloca) with a dbg.declare and the value loaded from the alloca with a dbg.value. Describing a function argument with a dbg.declare is undocumented in the LLVM-CFE contract and broke after LLVM r642022.

This patch just generates the alloca unconditionally, the mem2reg pass will eliminate it at -O1 and up anyway and points the dbg.declare to the alloca as intended (which mem2reg will then correctly rewrite into a dbg.value).

rdar://problem/35043980

[rjmccall]

The original code doesn't make any sense; it looks like the indirection is backwards. We just built a debug variable corresponding to the block descriptor pointer parameter, so LocalAddr should be dbg.declare'd to be that variable and Arg should be dbg.value'd. It looks like the real fix in your patch is just getting that right rather than anything to do with the alloca.

I don't particularly care about eliminating the alloca, since there are plenty of other places where we emit allocas that we assume we can destroy, and a few easily-eliminated instructions in a scattered function isn't going to greatly contribute to -O build times. So I think this patch is fine.

[aprantl]

In D39305#908208, @rjmccall wrote:

The original code doesn't make any sense; it looks like the indirection is backwards. We just built a debug variable corresponding to the block descriptor pointer parameter, so LocalAddr should be dbg.declare'd to be that variable and Arg should be dbg.value'd. It looks like the real fix in your patch is just getting that right rather than anything to do with the alloca.

Correct. The alloca is just a simplification.

I don't particularly care about eliminating the alloca, since there are plenty of other places where we emit allocas that we assume we can destroy, and a few easily-eliminated instructions in a scattered function isn't going to greatly contribute to -O build times. So I think this patch is fine.

Thanks!

cfe/trunk/lib/CodeGen/CGBlocks.cpp

 }
 
 void CodeGenFunction::setBlockContextParameter(const ImplicitParamDecl *D,
                                                unsigned argNum,
                                                llvm::Value *arg) {
   assert(BlockInfo && "not emitting prologue of block invocation function?!");
 
   llvm::Value *localAddr = nullptr;
-  if (CGM.getCodeGenOpts().OptimizationLevel == 0) {
-    // Allocate a stack slot to let the debug info survive the RA.
-    Address alloc = CreateMemTemp(D->getType(), D->getName() + ".addr");
-    Builder.CreateStore(arg, alloc);
-    localAddr = Builder.CreateLoad(alloc);
-  }
+  // Allocate a stack slot like for any local variable to guarantee optimal
+  // debug info at -O0. The mem2reg pass will eliminate it when optimizing.
 
   if (CGDebugInfo *DI = getDebugInfo()) {
     if (CGM.getCodeGenOpts().getDebugInfo() >=
         codegenoptions::LimitedDebugInfo) {
       DI->setLocation(D->getLocation());
-      DI->EmitDeclareOfBlockLiteralArgVariable(*BlockInfo, arg, argNum,
-                                               localAddr, Builder);
+      DI->EmitDeclareOfBlockLiteralArgVariable(
+          *BlockInfo, D->getName(), argNum,
+          cast<llvm::AllocaInst>(alloc.getPointer()), Builder);
     }
   }
 
   SourceLocation StartLoc = BlockInfo->getBlockExpr()->getBody()->getLocStart();
   ApplyDebugLocation Scope(*this, StartLoc);
 
   // Instead of messing around with LocalDeclMap, just set the value
   // directly as BlockPointer.

cfe/trunk/lib/CodeGen/CGDebugInfo.h

public:
   /// Emit call to \c llvm.dbg.declare for an argument variable
   /// declaration.
   void EmitDeclareOfArgVariable(const VarDecl *Decl, llvm::Value *AI,
                                 unsigned ArgNo, CGBuilderTy &Builder);
 
   /// Emit call to \c llvm.dbg.declare for the block-literal argument
   /// to a block invocation function.
   void EmitDeclareOfBlockLiteralArgVariable(const CGBlockInfo &block,
-                                            llvm::Value *Arg, unsigned ArgNo,
-                                            llvm::Value *LocalAddr,
+                                            StringRef Name, unsigned ArgNo,
+                                            llvm::AllocaInst *LocalAddr,
                                             CGBuilderTy &Builder);
 
   /// Emit information about a global variable.
   void EmitGlobalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
 
   /// Emit a constant global variable's debug info.
   void EmitGlobalVariable(const ValueDecl *VD, const APValue &Init);
 

cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

struct BlockLayoutChunk {
   const BlockDecl::Capture *Capture;
 };
 bool operator<(const BlockLayoutChunk &l, const BlockLayoutChunk &r) {
   return l.OffsetInBits < r.OffsetInBits;
 }
 }
 
 void CGDebugInfo::EmitDeclareOfBlockLiteralArgVariable(const CGBlockInfo &block,
-                                                       llvm::Value *Arg,
+                                                       StringRef Name,
                                                        unsigned ArgNo,
-                                                       llvm::Value *LocalAddr,
+                                                       llvm::AllocaInst *Alloca,
                                                        CGBuilderTy &Builder) {
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   ASTContext &C = CGM.getContext();
   const BlockDecl *blockDecl = block.getBlockDecl();
 
   // Collect some general information about the block's location.
   SourceLocation loc = blockDecl->getCaretLocation();
   llvm::DIFile *tunit = getOrCreateFile(loc);
void CGDebugInfo::EmitDeclareOfBlockLiteralArgVariable(const CGBlockInfo &block,
   type = DBuilder.createPointerType(type, CGM.PointerWidthInBits);
 
   // Get overall information about the block.
   llvm::DINode::DIFlags flags = llvm::DINode::FlagArtificial;
   auto *scope = cast<llvm::DILocalScope>(LexicalBlockStack.back());
 
   // Create the descriptor for the parameter.
   auto *debugVar = DBuilder.createParameterVariable(
-      scope, Arg->getName(), ArgNo, tunit, line, type,
+      scope, Name, ArgNo, tunit, line, type,
       CGM.getLangOpts().Optimize, flags);
 
-  if (LocalAddr) {
-    // Insert an llvm.dbg.value into the current block.
-    DBuilder.insertDbgValueIntrinsic(
-        LocalAddr, debugVar, DBuilder.createExpression(),
-        llvm::DebugLoc::get(line, column, scope, CurInlinedAt),
-        Builder.GetInsertBlock());
-  }
-
   // Insert an llvm.dbg.declare into the current block.
-  DBuilder.insertDeclare(Arg, debugVar, DBuilder.createExpression(),
+  DBuilder.insertDeclare(Alloca, debugVar, DBuilder.createExpression(),
                          llvm::DebugLoc::get(line, column, scope, CurInlinedAt),
                          Builder.GetInsertBlock());
 }
 
 llvm::DIDerivedType *
 CGDebugInfo::getOrCreateStaticDataMemberDeclarationOrNull(const VarDecl *D) {
   if (!D->isStaticDataMember())
     return nullptr;

cfe/trunk/test/CodeGen/debug-info-block-vars.c

+// RUN: %clang_cc1 -x c -fblocks -debug-info-kind=standalone -emit-llvm -O0 \
+// RUN:   -triple x86_64-apple-darwin -o - %s | FileCheck %s
+// RUN: %clang_cc1 -x c -fblocks -debug-info-kind=standalone -emit-llvm -O1 \
+// RUN:   -triple x86_64-apple-darwin -o - %s \
+// RUN:   | FileCheck --check-prefix=CHECK-OPT %s
+
+// CHECK: define internal void @__f_block_invoke(i8* %.block_descriptor)
+// CHECK: %.block_descriptor.addr = alloca i8*, align 8
+// CHECK: %block.addr = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor* }>*, align 8
+// CHECK: store i8* %.block_descriptor, i8** %.block_descriptor.addr, align 8
+// CHECK: call void @llvm.dbg.declare(metadata i8** %.block_descriptor.addr,
+// CHECK-SAME:                        metadata !DIExpression())
+// CHECK-OPT-NOT: alloca
+// CHECK-OPT: call void @llvm.dbg.value(metadata i8* %.block_descriptor,
+// CHECK-OPT-SAME:                      metadata !DIExpression())
+void f() {
+  a(^{
+    b();
+  });
+}

cfe/trunk/test/CodeGenObjC/debug-info-block-captured-self.m

-// RUN: %clang_cc1 -fblocks -debug-info-kind=limited -emit-llvm -triple x86_64-apple-darwin -o - %s | FileCheck %s
-//
-// Test that debug location is generated for a captured "self" inside
-// a block.
-//
-// This test is split into two parts, this one for the frontend, and
-// then llvm/test/DebugInfo/debug-info-block-captured-self.ll to
-// ensure that DW_AT_location is generated for the captured self.
-@class T;
-@interface S
-@end
-@interface Mode
--(int) count;
-@end
-@interface Context
-@end
-@interface ViewController
-@property (nonatomic, readwrite, strong) Context *context;
-@end
-typedef enum {
-    Unknown = 0,
-} State;
-@interface Main : ViewController
-{
-    T * t1;
-    T * t2;
-}
-@property(readwrite, nonatomic) State state;
-@end
-@implementation Main
-- (id) initWithContext:(Context *) context
-{
-    t1 = [self.context withBlock:^(id obj){
-        id *mode1;
-  t2 = [mode1 withBlock:^(id object){
-      Mode *mode2 = object;
-      if ([mode2 count] != 0) {
-        self.state = 0;
-      }
-    }];
-      }];
-}
-@end
-// The important part of this test is that there is a dbg.value
-// intrinsic associated with the implicit .block_descriptor argument
-// of the block. We also test that this value gets alloca'd, so the
-// register llocator won't accidentally kill it.
-
-// outer block:
-// CHECK: define internal void {{.*}}_block_invoke{{.*}}
-
-// inner block:
-// CHECK: define internal void {{.*}}_block_invoke{{.*}}
-// CHECK:        %[[MEM1:.*]] = alloca i8*, align 8
-// CHECK-NEXT:   %[[MEM2:.*]] = alloca i8*, align 8
-// CHECK-NEXT:   [[DBGADDR:%.*]] = alloca [[BLOCK_T:<{.*}>]]*, align 8
-// CHECK:        store i8* [[BLOCK_DESC:%.*]], i8** %[[MEM1]], align 8
-// CHECK:        %[[TMP0:.*]] = load i8*, i8** %[[MEM1]]
-// CHECK:        call void @llvm.dbg.value(metadata i8* %[[TMP0]], metadata ![[BDMD:[0-9]+]], metadata !{{.*}})
-// CHECK:        call void @llvm.dbg.declare(metadata i8* [[BLOCK_DESC]], metadata ![[BDMD:[0-9]+]], metadata !{{.*}})
-// CHECK:        store [[BLOCK_T]]* {{%.*}}, [[BLOCK_T]]** [[DBGADDR]], align 8
-// CHECK:        call void @llvm.dbg.declare(metadata [[BLOCK_T]]** [[DBGADDR]], metadata ![[SELF:.*]], metadata !{{.*}})
-// make sure we are still in the same function
-// CHECK: define {{.*}}__copy_helper_block_
-// Metadata
-// CHECK: ![[MAIN:.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Main"
-// CHECK-SAME:                            line: 23,
-// CHECK: ![[PMAIN:.*]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[MAIN]],
-// CHECK: ![[BDMD]] = !DILocalVariable(name: ".block_descriptor", arg:
-// CHECK: ![[SELF]] = !DILocalVariable(name: "self"
-// CHECK-NOT:                          arg:
-// CHECK-SAME:                         line: 40,