[DebugInfo] Add address space when creating DIDerivedTypes

[arsenm]

lib/Basic/Targets.cpp

2270

Should this be constant in some contexts?

[dblaikie]

test/CodeGenOpenCL/debugger-pointer-address-space.cl

1

Seems like a lot of test coverage for 4 changes. Is it all needed? I'd expect roughly 4 tests for these 4 changes. One for each that demonstrates the addr space is plumbed through. Probably no need to test that all addr spaces work in each context - because this code is pretty/entirely orthogonal to the particular address space.

[arsenm]

test/CodeGenOpenCL/debugger-pointer-address-space.cl

1

I think it's better to explicitly test all address spaces the target cares about

[frej]

lib/Basic/Targets.cpp

2269

Missing override

[kzhuravl]

Address review feedback:

• Add target hook for asking about LLVM AS -> DWARF AS
• Use Optional<unsigned> for DWARF AS

[kzhuravl]

lib/Basic/Targets.cpp

2270

I can't think of any at this moment.

[dblaikie]

include/clang/Basic/TargetInfo.h

1043

Is this a reasonable default implementation? Should there be a default or should any implementation (mabe only those with non-zero address spaces?) be expected to provide a mapping? (not sure how this looks for other things in TargetInfo)

[tony-tye]

include/clang/Basic/TargetInfo.h

1040–1041

Perhaps give a more detailed description:

"\return If a target requires an address within a target specific address space \p AddressSpace to be converted in order to be used, then return the corresponding target specific DWARF address space.
\return Otherwise return None and no conversion will be emitted in the DWARF."

lib/Basic/Targets.cpp

2259–2260

Same comment as above.

[kzhuravl]

Address review feedback

include/clang/Basic/TargetInfo.h

1043

I think the proposed hook is flexible (targets can map address spaces however they like) and returning None is a reasonable default (dwarf spec does not say that DW_AT_address_class is required - returning None here, will cause AsmPrinter not to generate DW_AT_address_class).

[kzhuravl]

AddressSpace -> DWARFAddressSpace.

[tony-tye]

lib/CodeGen/CGDebugInfo.cpp

1612

Should this be using the Constant address space since presumably a vtable will be in constant memory and not change during the execution of the program? This allows better code generation as constant data can be loaded by scalar operations on GPU hardware.

1653

Same comment as above.

[kzhuravl]

Address review feedback.

[tony-tye]

LGTM

[frej]

lib/Basic/Targets.cpp

2252

This one is missing an 'override'

[kzhuravl]

Add missing override keyword.

include/clang/Basic/TargetInfo.h

const OpenCLOptions &getSupportedOpenCLOpts() const {
       return getTargetOpts().SupportedOpenCLOptions;
   }
 
   /// \brief Get OpenCL image type address space.
   virtual LangAS::ID getOpenCLImageAddrSpace() const {
     return LangAS::opencl_global;
   }
 
+  /// \brief Get address space to use for global variables.
+  virtual unsigned getGlobalAddrSpace() const {
+    return 0;
+  }
+
   /// \brief Check the target is valid after it is fully initialized.
   virtual bool validateTarget(DiagnosticsEngine &Diags) const {
     return true;
   }
 
 protected:
   virtual uint64_t getPointerWidthV(unsigned AddrSpace) const {
     return PointerWidth;
   }
   virtual uint64_t getPointerAlignV(unsigned AddrSpace) const {
     return PointerAlign;
   }

lib/Basic/Targets.cpp

void setSupportedOpenCLOpts() override {
     }
   }
 
   LangAS::ID getOpenCLImageAddrSpace() const override {
     return LangAS::opencl_constant;
   }
 
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
     switch (CC) {
       default:
         return CCCR_Warning;
       case CC_C:
       case CC_OpenCLKernel:
         return CCCR_OK;
     }
   }
 
   // In amdgcn target the null pointer in global, constant, and generic
   // address space has value 0 but in private and local address space has
   // value ~0.
   uint64_t getNullPointerValue(unsigned AS) const override {
     return AS != LangAS::opencl_local && AS != 0 ? 0 : ~0;
   }
+
+  /// \brief Get address space to use for global variables.
+  unsigned getGlobalAddrSpace() const {
+    return 1; // opencl_global.
+  }
 };
 
 const Builtin::Info AMDGPUTargetInfo::BuiltinInfo[] = {
 #define BUILTIN(ID, TYPE, ATTRS)                \
   { #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr },
 #define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE)                               \
   { #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, FEATURE },
 #include "clang/Basic/BuiltinsAMDGPU.def"

lib/CodeGen/CGDebugInfo.cpp

llvm::DIType *CGDebugInfo::CreatePointerLikeType(llvm::dwarf::Tag Tag,
   // because that does not return the correct value for references.
   unsigned AS = CGM.getContext().getTargetAddressSpace(PointeeTy);
   uint64_t Size = CGM.getTarget().getPointerWidth(AS);
   auto Align = getTypeAlignIfRequired(Ty, CGM.getContext());
 
   if (Tag == llvm::dwarf::DW_TAG_reference_type ||
       Tag == llvm::dwarf::DW_TAG_rvalue_reference_type)
     return DBuilder.createReferenceType(Tag, getOrCreateType(PointeeTy, Unit),
-                                        Size, Align);
+                                        Size, Align, AS);
   else
     return DBuilder.createPointerType(getOrCreateType(PointeeTy, Unit), Size,
-                                      Align);
+                                      Align, AS);
 }
 
 llvm::DIType *CGDebugInfo::getOrCreateStructPtrType(StringRef Name,
                                                     llvm::DIType *&Cache) {
   if (Cache)
     return Cache;
   Cache = DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type, Name,
                                      TheCU, getOrCreateMainFile(), 0);
llvm::DIType *CGDebugInfo::getOrCreateVTablePtrType(llvm::DIFile *Unit) {
 
   ASTContext &Context = CGM.getContext();
 
   /* Function type */
   llvm::Metadata *STy = getOrCreateType(Context.IntTy, Unit);
   llvm::DITypeRefArray SElements = DBuilder.getOrCreateTypeArray(STy);
   llvm::DIType *SubTy = DBuilder.createSubroutineType(SElements);
   unsigned Size = Context.getTypeSize(Context.VoidPtrTy);
+  unsigned AS = CGM.getTarget().getGlobalAddrSpace();
   llvm::DIType *vtbl_ptr_type =
-      DBuilder.createPointerType(SubTy, Size, 0, "__vtbl_ptr_type");
+      DBuilder.createPointerType(SubTy, Size, 0, AS, "__vtbl_ptr_type");
   VTablePtrType = DBuilder.createPointerType(vtbl_ptr_type, Size);
   return VTablePtrType;
 }
 
 StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) {
   // Copy the gdb compatible name on the side and use its reference.
   return internString("_vptr$", RD->getNameAsString());
 }
void CGDebugInfo::CollectVTableInfo(const CXXRecordDecl *RD, llvm::DIFile *Unit,
   if (NeedVTableShape) {
     uint64_t PtrWidth =
         CGM.getContext().getTypeSize(CGM.getContext().VoidPtrTy);
     const VTableLayout &VFTLayout =
         CGM.getMicrosoftVTableContext().getVFTableLayout(RD, CharUnits::Zero());
     unsigned VSlotCount =
         VFTLayout.vtable_components().size() - CGM.getLangOpts().RTTIData;
     unsigned VTableWidth = PtrWidth * VSlotCount;
+    unsigned AS = CGM.getTarget().getGlobalAddrSpace();
 
     // Create a very wide void* type and insert it directly in the element list.
-    llvm::DIType *VTableType =
-        DBuilder.createPointerType(nullptr, VTableWidth, 0, "__vtbl_ptr_type");
+    llvm::DIType *VTableType = DBuilder.createPointerType(
+        nullptr, VTableWidth, 0, AS,"__vtbl_ptr_type");
     EltTys.push_back(VTableType);
 
     // The vptr is a pointer to this special vtable type.
     VPtrTy = DBuilder.createPointerType(VTableType, PtrWidth);
   }
 
   // If there is a primary base then the artificial vptr member lives there.
   if (RL.getPrimaryBase())

test/CodeGenOpenCL/debugger-pointer-address-space.cl

+// RUN: %clang -cl-std=CL2.0 -emit-llvm -g -O0 -S -target amdgcn-amd-amdhsa -mcpu=fiji -o - %s | FileCheck %s
+
+// CHECK-DAG: ![[ADDRSPACE1:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, addressSpace: 1)
+// CHECK-DAG: ![[ADDRSPACE2:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, addressSpace: 2)
+// CHECK-DAG: ![[ADDRSPACE3:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, addressSpace: 3)
+// CHECK-DAG: ![[ADDRSPACE4:[0-9]+]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !{{[0-9]+}}, size: {{[0-9]+}}, addressSpace: 4)
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar0", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE1]], isLocal: false, isDefinition: true)
+global int *FileVar0;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE2]], isLocal: false, isDefinition: true)
+constant int *FileVar1;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE3]], isLocal: false, isDefinition: true)
+local int *FileVar2;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar3", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE4]], isLocal: false, isDefinition: true)
+int *FileVar3;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar4", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE1]], isLocal: false, isDefinition: true)
+global int *global FileVar4;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar5", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE2]], isLocal: false, isDefinition: true)
+constant int *global FileVar5;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar6", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE3]], isLocal: false, isDefinition: true)
+local int *global FileVar6;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar7", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE4]], isLocal: false, isDefinition: true)
+int *global FileVar7;
+
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar8", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE1]], isLocal: false, isDefinition: true)
+global int *constant FileVar8 = 0;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar9", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE2]], isLocal: false, isDefinition: true)
+constant int *constant FileVar9 = 0;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar10", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE3]], isLocal: false, isDefinition: true)
+local int *constant FileVar10 = 0;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar11", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE4]], isLocal: false, isDefinition: true)
+int *constant FileVar11 = 0;
+
+// CHECK-DAG: !DILocalVariable(name: "KernelArg0", arg: 1, scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE1]])
+// CHECK-DAG: !DILocalVariable(name: "KernelArg1", arg: 2, scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE2]])
+// CHECK-DAG: !DILocalVariable(name: "KernelArg2", arg: 3, scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE3]])
+kernel void kernel1(global int *KernelArg0,
+                    constant int *KernelArg1,
+                    local int *KernelArg2) {
+  int *Tmp;
+
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar0", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE1]])
+  global int *FuncVar0 = KernelArg0;
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE2]])
+  constant int *FuncVar1 = KernelArg1;
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE3]])
+  local int *FuncVar2 = KernelArg2;
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar3", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE4]])
+  int *FuncVar3 = Tmp;
+
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar4", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE1]])
+  global int *constant FuncVar4 = KernelArg0;
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar5", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE2]])
+  constant int *constant FuncVar5 = KernelArg1;
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar6", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE3]])
+  local int *constant FuncVar6 = KernelArg2;
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar7", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE4]])
+  int *constant FuncVar7 = Tmp;
+
+  // CHECK-DAG: distinct !DIGlobalVariable(name: "FuncVar8", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE1]], isLocal: true, isDefinition: true)
+  global int *local FuncVar8; FuncVar8 = KernelArg0;
+  // CHECK-DAG: distinct !DIGlobalVariable(name: "FuncVar9", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE2]], isLocal: true, isDefinition: true)
+  constant int *local FuncVar9; FuncVar9 = KernelArg1;
+  // CHECK-DAG: distinct !DIGlobalVariable(name: "FuncVar10", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE3]], isLocal: true, isDefinition: true)
+  local int *local FuncVar10; FuncVar10 = KernelArg2;
+   // CHECK-DAG: distinct !DIGlobalVariable(name: "FuncVar11", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE4]], isLocal: true, isDefinition: true)
+  int *local FuncVar11; FuncVar11 = Tmp;
+
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar12", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE1]])
+  global int *private FuncVar12 = KernelArg0;
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar13", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE2]])
+  constant int *private FuncVar13 = KernelArg1;
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar14", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE3]])
+  local int *private FuncVar14 = KernelArg2;
+  // CHECK-DAG: !DILocalVariable(name: "FuncVar15", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[ADDRSPACE4]])
+  int *private FuncVar15 = Tmp;
+}
+
+struct FileStruct0 {
+  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "StructMem0", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, baseType: ![[ADDRSPACE1]], size: {{[0-9]+}})
+  global int *StructMem0;
+  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "StructMem1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, baseType: ![[ADDRSPACE2]], size: {{[0-9]+}}, offset: {{[0-9]+}})
+  constant int *StructMem1;
+  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "StructMem2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, baseType: ![[ADDRSPACE3]], size: {{[0-9]+}}, offset: {{[0-9]+}})
+  local int *StructMem2;
+  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "StructMem3", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, baseType: ![[ADDRSPACE4]], size: {{[0-9]+}}, offset: {{[0-9]+}})
+  int *StructMem3;
+};
+
+struct FileStruct1 {
+  union {
+    // CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "UnionMem0", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, baseType: ![[ADDRSPACE1]], size: {{[0-9]+}})
+    global int *UnionMem0;
+    // CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "UnionMem1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, baseType: ![[ADDRSPACE2]], size: {{[0-9]+}})
+    constant int *UnionMem1;
+    // CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "UnionMem2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, baseType: ![[ADDRSPACE3]], size: {{[0-9]+}})
+    local int *UnionMem2;
+    // CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "UnionMem3", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, baseType: ![[ADDRSPACE4]], size: {{[0-9]+}})
+    int *UnionMem3;
+  };
+  long StructMem0;
+};
+
+kernel void kernel2(global struct FileStruct0 *Kernel2Arg0,
+                    global struct FileStruct1 *Kernel2Arg1) {}