From 682774a09da2380b9f069291a3c6a937c20eaa9e Mon Sep 17 00:00:00 2001
From: Tony Tye <Tony.Tye@amd.com>
Date: Sun, 10 Jan 2021 03:16:12 +0000
Subject: [PATCH] Refine sampler handling

- Use std::unique_ptr to levarage RAII for managing the allocation in
  the presence of errors.

Change-Id: I55de515bbf72938e1dd09731c5e51f538cf9d34a
---
 rocclr/device/gpu/gpuprogram.cpp | 10 ++++++----
 rocclr/device/pal/palprogram.cpp | 18 +++++++++++-------
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/rocclr/device/gpu/gpuprogram.cpp b/rocclr/device/gpu/gpuprogram.cpp
index 391fc3ddc4..bd95dc8368 100644
--- a/rocclr/device/gpu/gpuprogram.cpp
+++ b/rocclr/device/gpu/gpuprogram.cpp
@@ -36,6 +36,7 @@
 #include <cstdio>
 #include <fstream>
 #include <iterator>
+#include <memory>
 #include <sstream>
 
 namespace gpu {
@@ -1977,6 +1978,7 @@ void* ORCAHSALoaderContext::SegmentAddress(amdgpu_hsa_elf_segment_t segment, hsa
 hsa_status_t ORCAHSALoaderContext::SamplerCreate(
     hsa_agent_t agent, const hsa_ext_sampler_descriptor_t* sampler_descriptor,
     hsa_ext_sampler_t* sampler_handle) {
+  sampler_handle->handle = 0;
   if (!agent.handle) {
     return HSA_STATUS_ERROR_INVALID_AGENT;
   }
@@ -1985,7 +1987,7 @@ hsa_status_t ORCAHSALoaderContext::SamplerCreate(
   }
 
   if (program_->isNull()) {
-    // Offline compilation. Provide a fake handle to avoid an assert
+    // Offline compilation. Provide a fake non-null handle.
     sampler_handle->handle = 1;
     return HSA_STATUS_SUCCESS;
   }
@@ -2033,13 +2035,12 @@ hsa_status_t ORCAHSALoaderContext::SamplerCreate(
       assert(false);
       return HSA_STATUS_ERROR_INVALID_ARGUMENT;
   }
-  gpu::Sampler* sampler = new gpu::Sampler(program_->gpuDevice());
+  std::unique_ptr<gpu::Sampler> sampler(new gpu::Sampler(program_->gpuDevice()));
   if (!sampler || !sampler->create(state)) {
-    delete sampler;
     return HSA_STATUS_ERROR;
   }
-  program_->addSampler(sampler);
   sampler_handle->handle = sampler->hwSrd();
+  program_->addSampler(sampler.release());
   return HSA_STATUS_SUCCESS;
 }
 
@@ -2051,6 +2052,7 @@ hsa_status_t ORCAHSALoaderContext::SamplerDestroy(hsa_agent_t agent,
   if (!sampler_handle.handle) {
     return HSA_STATUS_ERROR_INVALID_ARGUMENT;
   }
+  // Samplers will be destroyed by the pal::HSAILProgam destructor.
   return HSA_STATUS_SUCCESS;
 }
 
diff --git a/rocclr/device/pal/palprogram.cpp b/rocclr/device/pal/palprogram.cpp
index 954ff9396b..d6afeef44f 100644
--- a/rocclr/device/pal/palprogram.cpp
+++ b/rocclr/device/pal/palprogram.cpp
@@ -31,6 +31,7 @@
 #include <algorithm>
 #include <cstdio>
 #include <fstream>
+#include <memory>
 #include <iterator>
 #include <sstream>
 
@@ -588,11 +589,12 @@ void* PALHSALoaderContext::SegmentAlloc(amdgpu_hsa_elf_segment_t segment, hsa_ag
     }
     return ptr;
   }
-  Segment* seg = new Segment();
-  if (seg != nullptr && !seg->alloc(*program_, segment, size, align, zero)) {
+
+  std::unique_ptr<Segment> seg(new Segment());
+  if (!seg || !seg->alloc(*program_, segment, size, align, zero)) {
     return nullptr;
   }
-  return seg;
+  return seg.release();
 }
 
 bool PALHSALoaderContext::SegmentCopy(amdgpu_hsa_elf_segment_t segment, hsa_agent_t agent,
@@ -655,11 +657,13 @@ hsa_status_t PALHSALoaderContext::SamplerCreate(
   if (!sampler_descriptor || !sampler_handle) {
     return HSA_STATUS_ERROR_INVALID_ARGUMENT;
   }
+
   if (program_->isNull()) {
-    // Offline compilation. Provide a fake handle to avoid an assert
+    // Offline compilation. Provide a fake non-null handle.
     sampler_handle->handle = 1;
     return HSA_STATUS_SUCCESS;
   }
+
   uint32_t state = 0;
   switch (sampler_descriptor->coordinate_mode) {
     case HSA_EXT_SAMPLER_COORDINATE_MODE_UNNORMALIZED:
@@ -703,13 +707,12 @@ hsa_status_t PALHSALoaderContext::SamplerCreate(
       assert(false);
       return HSA_STATUS_ERROR_INVALID_ARGUMENT;
   }
-  pal::Sampler* sampler = new pal::Sampler(program_->palDevice());
+  std::unique_ptr<pal::Sampler> sampler(new pal::Sampler(program_->palDevice()));
   if (!sampler || !sampler->create(state)) {
-    delete sampler;
     return HSA_STATUS_ERROR;
   }
-  program_->addSampler(sampler);
   sampler_handle->handle = sampler->hwSrd();
+  program_->addSampler(sampler.release());
   return HSA_STATUS_SUCCESS;
 }
 
@@ -721,6 +724,7 @@ hsa_status_t PALHSALoaderContext::SamplerDestroy(hsa_agent_t agent,
   if (!sampler_handle.handle) {
     return HSA_STATUS_ERROR_INVALID_ARGUMENT;
   }
+  // Samplers will be destroyed by the pal::HSAILProgam destructor.
   return HSA_STATUS_SUCCESS;
 }