[Clang][NFC] Clang CUDA codegen clean-up

[bondhugula]

Summary

Update an instance of dyn_cast -> cast and other NFC clang-tidy fixes
for Clang CUDA codegen.

[bondhugula]

Drop redundant cast.

[bondhugula]

Gentle ping to reviewers @tra @yaxunl on this NFC PR.

[tra]

The description is a bit misleading. The dyn_cast->cast appears to be a minor part of this patch.
I'd separate clang-tidy cleanups into a separate patch or would just describe all of these changes as a clean-up. dyn_cast->cast is an NFC change here, too, considering that we're operating on a type of FunctionDecl *cudaLaunchKernelFD.

clang/lib/CodeGen/CGCUDANV.cpp

240–243

Nit: you could remove these {...} now, too.

[bondhugula]

In D112284#3085086, @tra wrote:

The description is a bit misleading. The dyn_cast->cast appears to be a minor part of this patch.
I'd separate clang-tidy cleanups into a separate patch or would just describe all of these changes as a clean-up. dyn_cast->cast is an NFC change here, too, considering that we're operating on a type of FunctionDecl *cudaLaunchKernelFD.

I agree. I've done the latter.

clang/lib/CodeGen/CGCUDANV.cpp

240–243

I retained this part due to the extra comment in line with LLVM style:
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
"We consider that readability is harmed when omitting the brace in the presence of a single statement that is accompanied by a comment (assuming the comment can’t be hoisted above the if or loop statement, see below)."

[bondhugula]

Address review comments. Update commit title and summary.

[bondhugula]

@tra While on this, I also wanted to ask as to why clang cuda codegen is using an argument on the global ctor and the dtor it's generating. The argument is unused and I couldn't find any code comments to support it in either CGCUDANV.cpp or CodeGenModule.cpp. I assume there is a reason because CodeGenModule.cpp is generating an explicit bitcast and has additional utilities to convert the function type to one with no arguments. Without the argument, one could just use appendToGlobalCtors/Dtors from ModuleUtils.

[tra]

In D112284#3086499, @bondhugula wrote:

@tra While on this, I also wanted to ask as to why clang cuda codegen is using an argument on the global ctor and the dtor it's generating.

It's a good question, and I don't have a good answer. It's quite possible that the parameter is not needed.

clang/lib/CodeGen/CGCUDANV.cpp

240–243

assuming the comment can’t be hoisted above the if

I think the comment can be hoisted up in this case. Up to you.

[bondhugula]

In D112284#3088531, @tra wrote:

In D112284#3086499, @bondhugula wrote:

@tra While on this, I also wanted to ask as to why clang cuda codegen is using an argument on the global ctor and the dtor it's generating.

It's a good question, and I don't have a good answer. It's quite possible that the parameter is not needed.

As a result of this parameter, there appears to be a lot of code in CodegenModule.cpp to add the ctor/dtor to the global lists. All of that would disappear with a single call to appendToGlobalCtors or appendToGlobalDtors if we got rid of the argument. Perhaps the extra argument was used initially for debugging or the fact that an extra argument bumps up the stack frame and makes something else work?

clang/lib/CodeGen/CGCUDANV.cpp

void handleVarRegistration(const VarDecl *VD,
                              llvm::GlobalVariable &Var) override;
   void
   internalizeDeviceSideVar(const VarDecl *D,
                            llvm::GlobalValue::LinkageTypes &Linkage) override;
 
   llvm::Function *finalizeModule() override;
 };
 
-}
+} // end anonymous namespace
 
 std::string CGNVCUDARuntime::addPrefixToName(StringRef FuncName) const {
   if (CGM.getLangOpts().HIP)
     return ((Twine("hip") + Twine(FuncName)).str());
   return ((Twine("cuda") + Twine(FuncName)).str());
 }
 std::string
 CGNVCUDARuntime::addUnderscoredPrefixToName(StringRef FuncName) const {
return CGM.CreateRuntimeFunction(
       addPrefixToName("SetupArgument"));
 }
 
 llvm::FunctionCallee CGNVCUDARuntime::getLaunchFn() const {
   if (CGM.getLangOpts().HIP) {
     // hipError_t hipLaunchByPtr(char *);
     return CGM.CreateRuntimeFunction(
         llvm::FunctionType::get(IntTy, CharPtrTy, false), "hipLaunchByPtr");
-  } else {
-    // cudaError_t cudaLaunch(char *);
-    return CGM.CreateRuntimeFunction(
-        llvm::FunctionType::get(IntTy, CharPtrTy, false), "cudaLaunch");
-  }
-}
+  }
 
 llvm::FunctionType *CGNVCUDARuntime::getRegisterGlobalsFnTy() const {
   return llvm::FunctionType::get(VoidTy, VoidPtrPtrTy, false);
 }
 
 llvm::FunctionType *CGNVCUDARuntime::getCallbackFnTy() const {
   return llvm::FunctionType::get(VoidTy, VoidPtrTy, false);
 }
 
 llvm::FunctionType *CGNVCUDARuntime::getRegisterLinkedBinaryFnTy() const {
-  auto CallbackFnTy = getCallbackFnTy();
-  auto RegisterGlobalsFnTy = getRegisterGlobalsFnTy();
+  auto *CallbackFnTy = getCallbackFnTy();
+  auto *RegisterGlobalsFnTy = getRegisterGlobalsFnTy();
   llvm::Type *Params[] = {RegisterGlobalsFnTy->getPointerTo(), VoidPtrTy,
                           VoidPtrTy, CallbackFnTy->getPointerTo()};
   return llvm::FunctionType::get(VoidTy, Params, false);
 }
 
 std::string CGNVCUDARuntime::getDeviceSideName(const NamedDecl *ND) {
   GlobalDecl GD;
   // D could be either a kernel or a variable.
void CGNVCUDARuntime::emitDeviceStubBodyNew(CodeGenFunction &CGF,
   LaunchKernelArgs.add(RValue::get(CGF.Builder.CreateLoad(ShmemSize)),
                        cudaLaunchKernelFD->getParamDecl(4)->getType());
   LaunchKernelArgs.add(RValue::get(CGF.Builder.CreateLoad(Stream)),
                        cudaLaunchKernelFD->getParamDecl(5)->getType());
 
   QualType QT = cudaLaunchKernelFD->getType();
   QualType CQT = QT.getCanonicalType();
   llvm::Type *Ty = CGM.getTypes().ConvertType(CQT);
-  llvm::FunctionType *FTy = dyn_cast<llvm::FunctionType>(Ty);
+  llvm::FunctionType *FTy = cast<llvm::FunctionType>(Ty);
 
   const CGFunctionInfo &FI =
       CGM.getTypes().arrangeFunctionDeclaration(cudaLaunchKernelFD);
   llvm::FunctionCallee cudaLaunchKernelFn =
       CGM.CreateRuntimeFunction(FTy, LaunchKernelName);
   CGF.EmitCall(FI, CGCallee::forDirect(cudaLaunchKernelFn), ReturnValueSlot(),
                LaunchKernelArgs);
   CGF.EmitBranch(EndBlock);
assert((!Var->isDeclaration() || Info.Flags.isManaged()) &&
            "External variables should not show up here, except HIP managed "
            "variables");
     llvm::Constant *VarName = makeConstantString(getDeviceSideName(Info.D));
     switch (Info.Flags.getKind()) {
     case DeviceVarFlags::Variable: {
       uint64_t VarSize =
           CGM.getDataLayout().getTypeAllocSize(Var->getValueType());
       if (Info.Flags.isManaged()) {
-        auto ManagedVar = new llvm::GlobalVariable(
+        auto *ManagedVar = new llvm::GlobalVariable(
             CGM.getModule(), Var->getType(),
             /*isConstant=*/false, Var->getLinkage(),
             /*Init=*/Var->isDeclaration()
                 ? nullptr
                 : llvm::ConstantPointerNull::get(Var->getType()),
             /*Name=*/"", /*InsertBefore=*/nullptr,
             llvm::GlobalVariable::NotThreadLocal);
         ManagedVar->setDSOLocal(Var->isDSOLocal());
if (IsHIP) {
     GpuBinaryHandle->setAlignment(CGM.getPointerAlign().getAsAlign());
     // Prevent the weak symbol in different shared libraries being merged.
     if (Linkage != llvm::GlobalValue::InternalLinkage)
       GpuBinaryHandle->setVisibility(llvm::GlobalValue::HiddenVisibility);
     Address GpuBinaryAddr(
         GpuBinaryHandle,
         CharUnits::fromQuantity(GpuBinaryHandle->getAlignment()));
     {
-      auto HandleValue = CtorBuilder.CreateLoad(GpuBinaryAddr);
+      auto *HandleValue = CtorBuilder.CreateLoad(GpuBinaryAddr);
       llvm::Constant *Zero =
           llvm::Constant::getNullValue(HandleValue->getType());
       llvm::Value *EQZero = CtorBuilder.CreateICmpEQ(HandleValue, Zero);
       CtorBuilder.CreateCondBr(EQZero, IfBlock, ExitBlock);
     }
     {
       CtorBuilder.SetInsertPoint(IfBlock);
       // GpuBinaryHandle = __hipRegisterFatBinary(&FatbinWrapper);
       llvm::CallInst *RegisterFatbinCall = CtorBuilder.CreateCall(
           RegisterFatbinFunc,
           CtorBuilder.CreateBitCast(FatbinWrapper, VoidPtrTy));
       CtorBuilder.CreateStore(RegisterFatbinCall, GpuBinaryAddr);
       CtorBuilder.CreateBr(ExitBlock);
     }
     {
       CtorBuilder.SetInsertPoint(ExitBlock);
       // Call __hip_register_globals(GpuBinaryHandle);
       if (RegisterGlobalsFunc) {
-        auto HandleValue = CtorBuilder.CreateLoad(GpuBinaryAddr);
+        auto *HandleValue = CtorBuilder.CreateLoad(GpuBinaryAddr);
         CtorBuilder.CreateCall(RegisterGlobalsFunc, HandleValue);
       }
     }
   } else if (!RelocatableDeviceCode) {
     // Register binary with CUDA runtime. This is substantially different in
     // default mode vs. separate compilation!
     // GpuBinaryHandle = __cudaRegisterFatBinary(&FatbinWrapper);
     llvm::CallInst *RegisterFatbinCall = CtorBuilder.CreateCall(
llvm::Function *CGNVCUDARuntime::makeModuleDtorFunction() {
 
   llvm::BasicBlock *DtorEntryBB =
       llvm::BasicBlock::Create(Context, "entry", ModuleDtorFunc);
   CGBuilderTy DtorBuilder(CGM, Context);
   DtorBuilder.SetInsertPoint(DtorEntryBB);
 
   Address GpuBinaryAddr(GpuBinaryHandle, CharUnits::fromQuantity(
                                              GpuBinaryHandle->getAlignment()));
-  auto HandleValue = DtorBuilder.CreateLoad(GpuBinaryAddr);
+  auto *HandleValue = DtorBuilder.CreateLoad(GpuBinaryAddr);
   // There is only one HIP fat binary per linked module, however there are
   // multiple destructor functions. Make sure the fat binary is unregistered
   // only once.
   if (CGM.getLangOpts().HIP) {
     llvm::BasicBlock *IfBlock =
         llvm::BasicBlock::Create(Context, "if", ModuleDtorFunc);
     llvm::BasicBlock *ExitBlock =
         llvm::BasicBlock::Create(Context, "exit", ModuleDtorFunc);

 // Each use of the original managed variable is replaced by a load from the
 // transformed managed variable. The transformed managed variable contains
 // the address of managed memory which will be allocated by the runtime.
 void CGNVCUDARuntime::transformManagedVars() {
   for (auto &&Info : DeviceVars) {
     llvm::GlobalVariable *Var = Info.Var;
     if (Info.Flags.getKind() == DeviceVarFlags::Variable &&
         Info.Flags.isManaged()) {
-      auto ManagedVar = new llvm::GlobalVariable(
+      auto *ManagedVar = new llvm::GlobalVariable(
           CGM.getModule(), Var->getType(),
           /*isConstant=*/false, Var->getLinkage(),
           /*Init=*/Var->isDeclaration()
               ? nullptr
               : llvm::ConstantPointerNull::get(Var->getType()),
           /*Name=*/"", /*InsertBefore=*/nullptr,
           llvm::GlobalVariable::NotThreadLocal,
           CGM.getContext().getTargetAddressSpace(LangAS::cuda_device));