From 48d5ad93cd6921de498a00421d696dba33fac7e4 Mon Sep 17 00:00:00 2001 From: Joseph Huber Date: Wed, 1 Mar 2023 15:02:00 -0600 Subject: [PATCH] [OpenMP][NFC] Clean up Twines and other issues in plugins Summary: Tihs patch is mostly NFC to fix some warning currently present in OpenMP offloading plugins. Specifically this mostly removes the use of Twine variables in favor of LLVM's small string. Twine variables are prone to use-after-free and this is a cleaner way to concatenate a string. --- llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h | 36 +++++++++++----------- .../plugins-nextgen/amdgpu/src/rtl.cpp | 2 +- .../common/PluginInterface/PluginInterface.cpp | 19 ++++++------ 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h b/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h index 9346406..bfac2d7 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h @@ -85,23 +85,23 @@ struct GV { /// For AMDGPU GPUs static constexpr GV AMDGPUGridValues64 = { - 256, // GV_Slot_Size - 64, // GV_Warp_Size + 256, // GV_Slot_Size + 64, // GV_Warp_Size (1 << 16), // GV_Max_Teams - 440, // GV_Default_Num_Teams - 896, // GV_SimpleBufferSize - 1024, // GV_Max_WG_Size, - 256, // GV_Default_WG_Size + 440, // GV_Default_Num_Teams + 896, // GV_SimpleBufferSize + 1024, // GV_Max_WG_Size, + 256, // GV_Default_WG_Size }; static constexpr GV AMDGPUGridValues32 = { - 256, // GV_Slot_Size - 32, // GV_Warp_Size + 256, // GV_Slot_Size + 32, // GV_Warp_Size (1 << 16), // GV_Max_Teams - 440, // GV_Default_Num_Teams - 896, // GV_SimpleBufferSize - 1024, // GV_Max_WG_Size, - 256, // GV_Default_WG_Size + 440, // GV_Default_Num_Teams + 896, // GV_SimpleBufferSize + 1024, // GV_Max_WG_Size, + 256, // GV_Default_WG_Size }; template constexpr const GV &getAMDGPUGridValues() { @@ -111,13 +111,13 @@ template constexpr const GV &getAMDGPUGridValues() { /// For Nvidia GPUs static constexpr GV NVPTXGridValues = { - 256, // GV_Slot_Size - 32, // GV_Warp_Size + 256, // GV_Slot_Size + 32, // GV_Warp_Size (1 << 16), // GV_Max_Teams - 3200, // GV_Default_Num_Teams - 896, // GV_SimpleBufferSize - 1024, // GV_Max_WG_Size - 128, // GV_Default_WG_Size + 3200, // GV_Default_Num_Teams + 896, // GV_SimpleBufferSize + 1024, // GV_Max_WG_Size + 128, // GV_Default_WG_Size }; } // namespace omp diff --git a/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp b/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp index 40c616c..99b45ad 100644 --- a/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp +++ b/openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp @@ -2634,7 +2634,7 @@ Error AMDGPUKernelTy::printLaunchInfoDetails(GenericDeviceTy &GenericDevice, // TODO set correctly once host services available auto HostCallRequired = false; INFO(OMP_INFOTYPE_PLUGIN_KERNEL, GenericDevice.getDeviceId(), - "SGN:%s ConstWGSize:%d args:%d teamsXthrds:(%4dX%4d) " + "SGN:%s ConstWGSize:%d args:%d teamsXthrds:(%4luX%4d) " "reqd:(%4dX%4d) lds_usage:%uB sgpr_count:%u vgpr_count:%u " "sgpr_spill_count:%u vgpr_spill_count:%u tripcount:%lu rpc:%d n:%s\n", getExecutionModeName(), ConstWGSize, ArgNum, NumGroups, ThreadsPerGroup, diff --git a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp index 1d4d906..6598357 100644 --- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp +++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp @@ -117,12 +117,12 @@ public: /* Default in GB */ 64) {} void saveImage(const char *Name, DeviceImageTy &Image) { - Twine ImageName = Twine(Name) + Twine(".image"); + SmallString<128> ImageName = {Name, ".image"}; std::error_code EC; - raw_fd_ostream OS(ImageName.str(), EC); + raw_fd_ostream OS(ImageName, EC); if (EC) report_fatal_error("Error saving image : " + StringRef(EC.message())); - if (auto TgtImageBitcode = Image.getTgtImageBitcode()) { + if (const auto *TgtImageBitcode = Image.getTgtImageBitcode()) { size_t Size = getPtrDiff(TgtImageBitcode->ImageEnd, TgtImageBitcode->ImageStart); MemoryBufferRef MBR = MemoryBufferRef( @@ -158,11 +158,10 @@ public: JsonArgOffsets.push_back(ArgOffsets[I]); JsonKernelInfo["ArgOffsets"] = json::Value(std::move(JsonArgOffsets)); - Twine KernelName(Name); - Twine MemoryFilename = KernelName + ".memory"; - dumpDeviceMemory(MemoryFilename.str(), AsyncInfoWrapper); + SmallString<128> MemoryFilename = {Name, ".memory"}; + dumpDeviceMemory(MemoryFilename, AsyncInfoWrapper); - Twine JsonFilename = KernelName + ".json"; + SmallString<128> JsonFilename = {Name, ".json"}; std::error_code EC; raw_fd_ostream JsonOS(JsonFilename.str(), EC); if (EC) @@ -174,9 +173,9 @@ public: void saveKernelOutputInfo(const char *Name, AsyncInfoWrapperTy &AsyncInfoWrapper) { - Twine OutputFilename = - Twine(Name) + (isRecording() ? ".original.output" : ".replay.output"); - dumpDeviceMemory(OutputFilename.str(), AsyncInfoWrapper); + SmallString<128> OutputFilename = { + Name, (isRecording() ? ".original.output" : ".replay.output")}; + dumpDeviceMemory(OutputFilename, AsyncInfoWrapper); } void *alloc(uint64_t Size) { -- 2.7.4