dynamic_loader: incorporate review feedback

src/CMakeLists.txt

@@ -43,6 +43,7 @@ add_library(migraphx
     cpp_generator.cpp
     dead_code_elimination.cpp
     dom_info.cpp
+    dynamic_loader.cpp
     eliminate_allocation.cpp
     eliminate_common_subexpression.cpp
     eliminate_concat.cpp
@@ -104,10 +105,7 @@ add_library(migraphx
     verify_args.cpp
 )
 if(WIN32)
-    target_sources(migraphx PRIVATE dynamic_loader_win32.cpp)
     target_compile_definitions(migraphx PUBLIC -D_USE_MATH_DEFINES)
-else()
-    target_sources(migraphx PRIVATE dynamic_loader.cpp)
 endif()
 configure_file(version.h.in include/migraphx/version.h)
 rocm_set_soversion(migraphx ${MIGRAPHX_SO_VERSION})

src/dynamic_loader.cpp

@@ -27,11 +27,20 @@
 #include <migraphx/file_buffer.hpp>
 #include <migraphx/tmp_dir.hpp>
 #include <utility>
+
+#ifdef _WIN32
+// cppcheck-suppress definePrefix
+#define WIN32_LEAN_AND_MEAN
+#include <Windows.h>
+#else
 #include <dlfcn.h>
+#endif
 
 namespace migraphx {
 inline namespace MIGRAPHX_INLINE_NS {
 
+#ifndef _WIN32
+
 void check_load_error(bool flush = false)
 {
     char* error_msg = dlerror();
@@ -81,6 +90,42 @@ fs::path dynamic_loader::path(void* address)
     return p;
 }
 
+#else
+
+struct dynamic_loader_impl
+{
+    dynamic_loader_impl() = default;
+    dynamic_loader_impl(const fs::path& p, tmp_dir t = {})
+        : handle{LoadLibrary(p.string().c_str())}, temp{std::move(t)}
+    {
+        if(handle == nullptr)
+        {
+            MIGRAPHX_THROW("Error loading DLL: " + p.string() + " (" +
+                           std::to_string(GetLastError()) + ")");
+        }
+    }
+
+    ~dynamic_loader_impl()
+    {
+        if(handle != nullptr)
+        {
+            FreeLibrary(handle);
+        }
+    }
+    static std::shared_ptr<dynamic_loader_impl> from_buffer(const char* image, std::size_t size)
+    {
+        auto t = tmp_dir{"migx-dynload"};
+        auto f = t.path / "tmp.dll";
+        write_buffer(f.string(), image, size);
+        return std::make_shared<dynamic_loader_impl>(f, std::move(t));
+    }
+
+    HMODULE handle = nullptr;
+    tmp_dir temp;
+};
+
+#endif
+
 optional<dynamic_loader> dynamic_loader::try_load(const fs::path& p)
 {
     try
@@ -109,12 +154,19 @@ dynamic_loader::dynamic_loader(const std::vector<char>& buffer)
 
 std::shared_ptr<void> dynamic_loader::get_symbol(const std::string& name) const
 {
+#ifndef _WIN32
     // flush any previous error messages
     check_load_error(true);
     void* symbol = dlsym(impl->handle.get(), name.c_str());
     if(symbol == nullptr)
         check_load_error();
     return {impl, symbol};
+#else
+    FARPROC addr = GetProcAddress(impl->handle, name.c_str());
+    if(addr == nullptr)
+        MIGRAPHX_THROW("Symbol not found: " + name + " (" + std::to_string(GetLastError()) + ")");
+    return {impl, reinterpret_cast<void*>(addr)};
+#endif
 }
 
 } // namespace MIGRAPHX_INLINE_NS

src/dynamic_loader_win32.cpp

-/*
- * The MIT License (MIT)
- *
- * Copyright (c) 2015-2022 Advanced Micro Devices, Inc. All rights reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-#include <migraphx/dynamic_loader.hpp>
-#include <migraphx/errors.hpp>
-#include <migraphx/file_buffer.hpp>
-#include <migraphx/tmp_dir.hpp>
-#include <utility>
-
-// cppcheck-suppress definePrefix
-#define WIN32_LEAN_AND_MEAN
-#include <Windows.h>
-
-namespace migraphx {
-inline namespace MIGRAPHX_INLINE_NS {
-
-struct dynamic_loader_impl
-{
-    dynamic_loader_impl() = default;
-    dynamic_loader_impl(const fs::path& p, std::shared_ptr<tmp_dir> t = nullptr)
-        : handle(LoadLibrary(p.string().c_str())), temp(std::move(t))
-    {
-        if(handle == nullptr)
-        {
-            MIGRAPHX_THROW("Error loading DLL: " + p.string() + " (" +
-                           std::to_string(GetLastError()) + ")");
-        }
-    }
-
-    ~dynamic_loader_impl()
-    {
-        if(handle != nullptr)
-        {
-            FreeLibrary(handle);
-        }
-    }
-    static std::shared_ptr<dynamic_loader_impl> from_buffer(const char* image, std::size_t size)
-    {
-        auto t = std::make_shared<tmp_dir>("migx-dynload");
-        auto f = t->path / "tmp.dll";
-        write_buffer(f.string(), image, size);
-        return std::make_shared<dynamic_loader_impl>(f, t);
-    }
-
-    HMODULE handle                = nullptr;
-    std::shared_ptr<tmp_dir> temp = nullptr;
-};
-
-optional<dynamic_loader> dynamic_loader::try_load(const fs::path& p)
-{
-    try
-    {
-        return dynamic_loader{p};
-    }
-    catch(const std::exception&)
-    {
-        return nullopt;
-    }
-}
-
-dynamic_loader::dynamic_loader(const fs::path& p) : impl(std::make_shared<dynamic_loader_impl>(p))
-{
-}
-
-dynamic_loader::dynamic_loader(const char* image, std::size_t size)
-    : impl(dynamic_loader_impl::from_buffer(image, size))
-{
-}
-
-dynamic_loader::dynamic_loader(const std::vector<char>& buffer)
-    : impl(dynamic_loader_impl::from_buffer(buffer.data(), buffer.size()))
-{
-}
-
-std::shared_ptr<void> dynamic_loader::get_symbol(const std::string& name) const
-{
-    FARPROC addr = GetProcAddress(impl->handle, name.c_str());
-    if(addr == nullptr)
-        MIGRAPHX_THROW("Symbol not found: " + name + " (" + std::to_string(GetLastError()) + ")");
-    return {impl, reinterpret_cast<void*>(addr)};
-}
-
-} // namespace MIGRAPHX_INLINE_NS
-} // namespace migraphx

src/include/migraphx/tmp_dir.hpp

@@ -34,6 +34,7 @@ struct MIGRAPHX_EXPORT tmp_dir
 {
     fs::path path;
     tmp_dir(const std::string& prefix = "");
+    tmp_dir(tmp_dir&&) = default;
 
     void execute(const std::string& exe, const std::string& args) const;