Do not append terminating NUL to the string with embedded GPU binary.

[tra]

Summary

Extra NUL does not impact functionality of the generated code, but it confuses
various NVIDIA tools used to examine embedded GPU binaries.

[efriedma]

clang/lib/CodeGen/CGCUDANV.cpp

92

Please don't abuse GetAddrOfConstantCString like this. Like the name says, it's meant for strings, in the normal sections strings would go in. If you just want an array global, please just use "new llvm::GlobalVariable" directly.

(AddNull itself is not a big deal, but messing with the section/alignment/unnamed_addr of globals in CodeGenModule's ConstantStringMap is a bad idea.)

[tra]

Reworked string/array generation.

[tra]

clang/lib/CodeGen/CGCUDANV.cpp

92

Agreed. That was not the best choice. I've separated generation of regular strings from generation of constant arrays we need to place just so.

[yaxunl]

LGTM. Thanks.

clang/lib/CodeGen/CGCUDANV.cpp

private:
   /// Keeps track of variable containing handle of GPU binary. Populated by
   /// ModuleCtorFunction() and used to create corresponding cleanup calls in
   /// ModuleDtorFunction()
   llvm::GlobalVariable *GpuBinaryHandle = nullptr;
   /// Whether we generate relocatable device code.
   bool RelocatableDeviceCode;
   /// Mangle context for device.
   std::unique_ptr<MangleContext> DeviceMC;
+  /// Some zeros used for GEPs.
+  llvm::Constant *Zeros[2];
 
   llvm::FunctionCallee getSetupArgumentFn() const;
   llvm::FunctionCallee getLaunchFn() const;
 
   llvm::FunctionType *getRegisterGlobalsFnTy() const;
   llvm::FunctionType *getCallbackFnTy() const;
   llvm::FunctionType *getRegisterLinkedBinaryFnTy() const;
   std::string addPrefixToName(StringRef FuncName) const;
   std::string addUnderscoredPrefixToName(StringRef FuncName) const;
 
   /// Creates a function to register all kernel stubs generated in this module.
   llvm::Function *makeRegisterGlobalsFn();
 
   /// Helper function that generates a constant string and returns a pointer to
   /// the start of the string.  The result of this function can be used anywhere
   /// where the C code specifies const char*.
   llvm::Constant *makeConstantString(const std::string &Str,
-                                     const std::string &Name = "",
-                                     const std::string &SectionName = "",
-                                     unsigned Alignment = 0) {
-    llvm::Constant *Zeros[] = {llvm::ConstantInt::get(SizeTy, 0),
-                               llvm::ConstantInt::get(SizeTy, 0)};
+                                     const std::string &Name = "") {
     auto ConstStr = CGM.GetAddrOfConstantCString(Str, Name.c_str());
-    llvm::GlobalVariable *GV =
-        cast<llvm::GlobalVariable>(ConstStr.getPointer());
+    return llvm::ConstantExpr::getGetElementPtr(ConstStr.getElementType(),
+                                                ConstStr.getPointer(), Zeros);
+  }
+
+  /// Helper function which generates an initialized constant array from Str,
+  /// and optionally sets section name and alignment. AddNull specifies whether
+  /// the array should nave NUL termination.
+  llvm::Constant *makeConstantArray(StringRef Str,
+                                    StringRef Name = "",
+                                    StringRef SectionName = "",
+                                    unsigned Alignment = 0,
+                                    bool AddNull = false) {
+    llvm::Constant *Value =
+        llvm::ConstantDataArray::getString(Context, Str, AddNull);
+    auto *GV = new llvm::GlobalVariable(
+        TheModule, Value->getType(), /*isConstant=*/true,
+        llvm::GlobalValue::PrivateLinkage, Value, Name);
     if (!SectionName.empty()) {
       GV->setSection(SectionName);
       // Mark the address as used which make sure that this section isn't
       // merged and we will really have it in the object file.
       GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::None);
     }
     if (Alignment)
       GV->setAlignment(llvm::Align(Alignment));
-
-    return llvm::ConstantExpr::getGetElementPtr(ConstStr.getElementType(),
-                                                ConstStr.getPointer(), Zeros);
+    return llvm::ConstantExpr::getGetElementPtr(GV->getValueType(), GV, Zeros);
   }
 
   /// Helper function that generates an empty dummy function returning void.
   llvm::Function *makeDummyFunction(llvm::FunctionType *FnTy) {
     assert(FnTy->getReturnType()->isVoidTy() &&
            "Can only generate dummy functions returning void!");
     llvm::Function *DummyFunc = llvm::Function::Create(
         FnTy, llvm::GlobalValue::InternalLinkage, "dummy", &TheModule);
: CGCUDARuntime(CGM), Context(CGM.getLLVMContext()),
       RelocatableDeviceCode(CGM.getLangOpts().GPURelocatableDeviceCode),
       DeviceMC(InitDeviceMC(CGM)) {
   CodeGen::CodeGenTypes &Types = CGM.getTypes();
   ASTContext &Ctx = CGM.getContext();
 
   IntTy = CGM.IntTy;
   SizeTy = CGM.SizeTy;
   VoidTy = CGM.VoidTy;
+  Zeros[0] = llvm::ConstantInt::get(SizeTy, 0);
+  Zeros[1] = Zeros[0];
 
   CharPtrTy = llvm::PointerType::getUnqual(Types.ConvertType(Ctx.CharTy));
   VoidPtrTy = cast<llvm::PointerType>(Types.ConvertType(Ctx.VoidPtrTy));
   VoidPtrPtrTy = VoidPtrTy->getPointerTo();
 }
 
 llvm::FunctionCallee CGNVCUDARuntime::getSetupArgumentFn() const {
   // cudaError_t cudaSetupArgument(void *, size_t, size_t)
if (IsHIP) {
 
     ModuleIDSectionName = "__hip_module_id";
     ModuleIDPrefix = "__hip_";
 
     if (CudaGpuBinary) {
       // If fatbin is available from early finalization, create a string
       // literal containing the fat binary loaded from the given file.
       const unsigned HIPCodeObjectAlign = 4096;
-      FatBinStr =
-          makeConstantString(std::string(CudaGpuBinary->getBuffer()), "",
+      FatBinStr = makeConstantArray(std::string(CudaGpuBinary->getBuffer()), "",
                                     FatbinConstantName, HIPCodeObjectAlign);
     } else {
       // If fatbin is not available, create an external symbol
       // __hip_fatbin in section .hip_fatbin. The external symbol is supposed
       // to contain the fat binary but will be populated somewhere else,
       // e.g. by lld through link script.
       FatBinStr = new llvm::GlobalVariable(
         CGM.getModule(), CGM.Int8Ty,
         /*isConstant=*/true, llvm::GlobalValue::ExternalLinkage, nullptr,
if (IsHIP) {
 
     ModuleIDSectionName = CGM.getTriple().isMacOSX()
                               ? "__NV_CUDA,__nv_module_id"
                               : "__nv_module_id";
     ModuleIDPrefix = "__nv_";
 
     // For CUDA, create a string literal containing the fat binary loaded from
     // the given file.
-    FatBinStr = makeConstantString(std::string(CudaGpuBinary->getBuffer()), "",
-                                   FatbinConstantName, 8);
+    FatBinStr = makeConstantArray(std::string(CudaGpuBinary->getBuffer()), "",
     FatMagic = CudaFatMagic;
   }
 
   // Create initialized wrapper structure that points to the loaded GPU binary
   ConstantInitBuilder Builder(CGM);
   auto Values = Builder.beginStruct(FatbinWrapperTy);
   // Fatbin wrapper magic.
   Values.addInt(IntTy, FatMagic);
if (CudaFeatureEnabled(CGM.getTarget().getSDKVersion(),
           "__cudaRegisterFatBinaryEnd");
       CtorBuilder.CreateCall(RegisterFatbinEndFunc, RegisterFatbinCall);
     }
   } else {
     // Generate a unique module ID.
     SmallString<64> ModuleID;
     llvm::raw_svector_ostream OS(ModuleID);
     OS << ModuleIDPrefix << llvm::format("%" PRIx64, FatbinWrapper->getGUID());
-    llvm::Constant *ModuleIDConstant = makeConstantString(
-        std::string(ModuleID.str()), "", ModuleIDSectionName, 32);
+    llvm::Constant *ModuleIDConstant = makeConstantArray(
+        std::string(ModuleID.str()), "", ModuleIDSectionName, 32, /*AddNull=*/true);
 
     // Create an alias for the FatbinWrapper that nvcc will look for.
     llvm::GlobalAlias::create(llvm::GlobalValue::ExternalLinkage,
                               Twine("__fatbinwrap") + ModuleID, FatbinWrapper);
 
     // void __cudaRegisterLinkedBinary%ModuleID%(void (*)(void *), void *,
     // void *, void (*)(void **))
     SmallString<128> RegisterLinkedBinaryName("__cudaRegisterLinkedBinary");

clang/test/CodeGenCUDA/device-stub.cu

-// RUN: echo "GPU binary would be here" > %t
+// RUN: echo -n "GPU binary would be here." > %t
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN:     -target-sdk-version=8.0 -fcuda-include-gpubinary %t -o - \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s \
 // RUN:       --check-prefixes=ALL,LNX,NORDC,CUDA,CUDANORDC,CUDA-OLD
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN:     -target-sdk-version=8.0  -fcuda-include-gpubinary %t \
 // RUN:     -o - -DNOGLOBALS \
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s \

 // ALL: @0 = private unnamed_addr constant [18 x i8] c"_Z10kernelfunciii\00"
 // * constant unnamed string with the device-side kernel name to be passed to
 //   __hipRegisterVar/__cudaRegisterVar.
 // ALL: @1 = private unnamed_addr constant [11 x i8] c"device_var\00"
 // ALL: @2 = private unnamed_addr constant [13 x i8] c"constant_var\00"
 // ALL: @3 = private unnamed_addr constant [19 x i8] c"ext_device_var_def\00"
 // ALL: @4 = private unnamed_addr constant [21 x i8] c"ext_constant_var_def\00"
 // * constant unnamed string with GPU binary
-// CUDA: @[[FATBIN:.*]] = private constant{{.*GPU binary would be here.*}}\00",
-// HIPEF: @[[FATBIN:.*]] = private constant{{.*GPU binary would be here.*}}\00",{{.*}}align 4096
+// CUDA: @[[FATBIN:.*]] = private constant{{.*}} c"GPU binary would be here.",
+// HIPEF: @[[FATBIN:.*]] = private constant{{.*}} c"GPU binary would be here.",{{.*}}align 4096
 // HIPNEF: @[[FATBIN:__hip_fatbin]] = external constant i8, section ".hip_fatbin"
 // CUDANORDC-SAME: section ".nv_fatbin", align 8
 // CUDARDC-SAME: section "__nv_relfatbin", align 8
 // * constant struct that wraps GPU binary
 // ALL: @__[[PREFIX:cuda|hip]]_fatbin_wrapper = internal constant
 // LNX-SAME: { i32, i32, i8*, i8* }
 // CUDA-SAME: { i32 1180844977, i32 1,
 // HIP-SAME: { i32 1212764230, i32 1,