[DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

[scott.linder]

Summary

OpenCL block literal structs have different fields which are now correctly identified in the debug info.

[yaxunl]

LGTM. Thanks.

[scott.linder]

Thank you for taking a look @yaxunl. Should I wait for another reviewer or can I commit this?

[yaxunl]

In D49930#1180388, @scott.linder wrote:

Thank you for taking a look @yaxunl. Should I wait for another reviewer or can I commit this?

I think it is OK to land. We could have post-commit reviews if there are any issues.

[echristo]

The patch is fine, in general, couple of comments:

a) Can you refactor this so the if conditionals are just two functions? Those functions are big enough already.
b) I'm not quite sure why you're picking limited here, do you have an explanation?
c) Can you split that part out into a separate test? Additional run lines in the existing blocks.cl test would be fine.

Thanks!

[scott.linder]

Sorry, I didn't see the additional comments until after I committed. I will make those changes; is it OK to update this review, or should I create a new one?

As for choosing limited it was just what the function adding the debug info checked for as a minimum; what would you generally choose instead?

[echristo]

In D49930#1181000, @scott.linder wrote:

Sorry, I didn't see the additional comments until after I committed. I will make those changes; is it OK to update this review, or should I create a new one?

A new one is great. Just treat this as post commit :)

As for choosing limited it was just what the function adding the debug info checked for as a minimum; what would you generally choose instead?

I'd have just suggested plain -g for it?

cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

auto *EltTy =
                                 FieldOffset, 0, Flags, nullptr, Elements);
 
   // Bit size, align and offset of the type.
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
 
   auto *DescTy = DBuilder.createPointerType(EltTy, Size);
 
   FieldOffset = 0;
+  if (CGM.getLangOpts().OpenCL) {
+    FType = CGM.getContext().IntTy;
+    EltTys.push_back(CreateMemberType(Unit, FType, "__size", &FieldOffset));
+    EltTys.push_back(CreateMemberType(Unit, FType, "__align", &FieldOffset));
+  } else {
     FType = CGM.getContext().getPointerType(CGM.getContext().VoidTy);
     EltTys.push_back(CreateMemberType(Unit, FType, "__isa", &FieldOffset));
     FType = CGM.getContext().IntTy;
     EltTys.push_back(CreateMemberType(Unit, FType, "__flags", &FieldOffset));
     EltTys.push_back(CreateMemberType(Unit, FType, "__reserved", &FieldOffset));
     FType = CGM.getContext().getPointerType(Ty->getPointeeType());
     EltTys.push_back(CreateMemberType(Unit, FType, "__FuncPtr", &FieldOffset));
-
     FType = CGM.getContext().getPointerType(CGM.getContext().VoidTy);
     FieldSize = CGM.getContext().getTypeSize(Ty);
     FieldAlign = CGM.getContext().getTypeAlign(Ty);
     EltTys.push_back(DBuilder.createMemberType(
         Unit, "__descriptor", nullptr, LineNo, FieldSize, FieldAlign, FieldOffset,
         llvm::DINode::FlagZero, DescTy));
+  }
 
   FieldOffset += FieldSize;
   Elements = DBuilder.getOrCreateArray(EltTys);
 
   // The __block_literal_generic structs are marked with a special
   // DW_AT_APPLE_BLOCK attribute and are an implementation detail only
   // the debugger needs to know about. To allow type uniquing, emit
   // them without a name or a location.
void CGDebugInfo::EmitDeclareOfBlockLiteralArgVariable(const CGBlockInfo &block,
 
   // Build the debug-info type for the block literal.
   getDeclContextDescriptor(blockDecl);
 
   const llvm::StructLayout *blockLayout =
       CGM.getDataLayout().getStructLayout(block.StructureType);
 
   SmallVector<llvm::Metadata *, 16> fields;
+  if (CGM.getLangOpts().OpenCL) {
+    fields.push_back(createFieldType("__size", C.IntTy, loc, AS_public,
+                                     blockLayout->getElementOffsetInBits(0),
+                                     tunit, tunit));
+    fields.push_back(createFieldType("__align", C.IntTy, loc, AS_public,
+                                     blockLayout->getElementOffsetInBits(1),
+                                     tunit, tunit));
+  } else {
     fields.push_back(createFieldType("__isa", C.VoidPtrTy, loc, AS_public,
                                      blockLayout->getElementOffsetInBits(0),
                                      tunit, tunit));
     fields.push_back(createFieldType("__flags", C.IntTy, loc, AS_public,
                                      blockLayout->getElementOffsetInBits(1),
                                      tunit, tunit));
     fields.push_back(createFieldType("__reserved", C.IntTy, loc, AS_public,
                                      blockLayout->getElementOffsetInBits(2),
                                      tunit, tunit));
     auto *FnTy = block.getBlockExpr()->getFunctionType();
     auto FnPtrType = CGM.getContext().getPointerType(FnTy->desugar());
     fields.push_back(createFieldType("__FuncPtr", FnPtrType, loc, AS_public,
                                      blockLayout->getElementOffsetInBits(3),
                                      tunit, tunit));
     fields.push_back(createFieldType(
         "__descriptor",
         C.getPointerType(block.NeedsCopyDispose
                              ? C.getBlockDescriptorExtendedType()
                              : C.getBlockDescriptorType()),
         loc, AS_public, blockLayout->getElementOffsetInBits(4), tunit, tunit));
+  }
 
   // We want to sort the captures by offset, not because DWARF
   // requires this, but because we're paranoid about debuggers.
   SmallVector<BlockLayoutChunk, 8> chunks;
 
   // 'this' capture.
   if (blockDecl->capturesCXXThis()) {
     BlockLayoutChunk chunk;

cfe/trunk/test/CodeGenOpenCL/blocks.cl

-// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -O0 -triple spir-unknown-unknown | FileCheck -check-prefixes=COMMON,SPIR %s
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -O0 -triple amdgcn-amd-amdhsa | FileCheck -check-prefixes=COMMON,AMDGCN %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -O0 -debug-info-kind=limited -triple spir-unknown-unknown | FileCheck -check-prefixes=COMMON,SPIR %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -O0 -debug-info-kind=limited -triple amdgcn-amd-amdhsa | FileCheck -check-prefixes=COMMON,AMDGCN %s
 
 // COMMON: @__block_literal_global = internal addrspace(1) constant { i32, i32 } { i32 8, i32 4 }
 // COMMON-NOT: .str
 
 // SPIR-LABEL: define internal {{.*}}void @block_A_block_invoke(i8 addrspace(4)* %.block_descriptor, i8 addrspace(3)* %a)
 // AMDGCN-LABEL: define internal {{.*}}void @block_A_block_invoke(i8* %.block_descriptor, i8 addrspace(3)* %a)
 void (^block_A)(local void *) = ^(local void *a) {
   return;

 // SPIR:  %[[block_capture_addr:.*]] = getelementptr inbounds <{ i32, i32, i32 }>, <{ i32, i32, i32 }> addrspace(4)* %[[block]], i32 0, i32 2
 // SPIR:  %[[block_capture:.*]] = load i32, i32 addrspace(4)* %[[block_capture_addr]]
 // AMDGCN-LABEL: define internal {{.*}}i32 @__foo_block_invoke(i8* %.block_descriptor)
 // AMDGCN:  %[[block:.*]] = bitcast i8* %.block_descriptor to <{ i32, i32, i32 }>*
 // AMDGCN:  %[[block_capture_addr:.*]] = getelementptr inbounds <{ i32, i32, i32 }>, <{ i32, i32, i32 }>* %[[block]], i32 0, i32 2
 // AMDGCN:  %[[block_capture:.*]] = load i32, i32* %[[block_capture_addr]]
 
 // COMMON-NOT: define{{.*}}@__foo_block_invoke_kernel
+
+// COMMON: !DIDerivedType(tag: DW_TAG_member, name: "__size"
+// COMMON: !DIDerivedType(tag: DW_TAG_member, name: "__align"
+
+// COMMON-NOT: !DIDerivedType(tag: DW_TAG_member, name: "__isa"
+// COMMON-NOT: !DIDerivedType(tag: DW_TAG_member, name: "__flags"
+// COMMON-NOT: !DIDerivedType(tag: DW_TAG_member, name: "__reserved"
+// COMMON-NOT: !DIDerivedType(tag: DW_TAG_member, name: "__FuncPtr"