[Openmp-commits] [openmp] 48d5ad9 - [OpenMP][NFC] Clean up Twines and other issues in plugins

[Joseph Huber via Openmp-commits]

Author: Joseph Huber
Date: 2023-03-01T15:03:21-06:00
New Revision: 48d5ad93cd6921de498a00421d696dba33fac7e4

URL: https://github.com/llvm/llvm-project/commit/48d5ad93cd6921de498a00421d696dba33fac7e4
DIFF: https://github.com/llvm/llvm-project/commit/48d5ad93cd6921de498a00421d696dba33fac7e4.diff

LOG: [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.

Added: 
    

Modified: 
    llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h
    openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
    openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h b/llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h
index 93464063dfaf5..bfac2d734b81d 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 <unsigned wavesize> constexpr const GV &getAMDGPUGridValues() {
@@ -111,13 +111,13 @@ template <unsigned wavesize> 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 40c616c249fdd..99b45ad386d8a 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 1d4d906bec4e8..65983577f08fd 100644
--- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
@@ -117,12 +117,12 @@ struct RecordReplayTy {
                               /* 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 @@ struct RecordReplayTy {
       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 @@ struct RecordReplayTy {
 
   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) {