Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix targets registration on Windows #2866

Merged
merged 35 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
34bf488
fix target registering on Windows
apwojcik Feb 27, 2024
fc29bd6
upgrade the year in licenses
apwojcik Mar 7, 2024
cd46541
fix compilation of the fpga target
Mar 7, 2024
60860e9
update the year in CMakeLists.txt files
Mar 7, 2024
7de05db
fix cppcheck reports
Mar 7, 2024
bbe1d3d
undo *requirements.txt
Mar 7, 2024
6b14d6d
Merge branch 'develop' into register_target
apwojcik Mar 7, 2024
1e1f4fb
minimize the number of changes required
apwojcik Mar 7, 2024
07cc374
add missing migraphx_all_targets to tests
apwojcik Mar 7, 2024
4624e43
update the year in the license
apwojcik Mar 7, 2024
151dac7
Merge branch 'develop' into register_target
apwojcik Apr 23, 2024
894d40f
Merge branch 'develop' into register_target
apwojcik May 8, 2024
45ede6f
incorporate review feedback
apwojcik May 8, 2024
caac2b0
fix typo
apwojcik May 8, 2024
0e73fdb
fix clang-format
apwojcik May 8, 2024
68c049c
Merge branch 'develop' into register_target
apwojcik May 8, 2024
f0d79d3
limit auto target registration to Windows only
apwojcik May 8, 2024
7090c2b
Merge branch 'develop' into register_target
apwojcik May 8, 2024
e1f6e2c
Merge branch 'develop' into register_target
apwojcik May 27, 2024
9d8bcf8
incorporate review feedback
apwojcik May 27, 2024
5e3411f
Merge branch 'develop' into register_target
apwojcik May 27, 2024
db78e91
incorporate review feedback
apwojcik May 27, 2024
5a235c1
fix license check
apwojcik May 27, 2024
6ec5a5e
fix clang-format
apwojcik May 27, 2024
31459c7
incorporate review feedback
apwojcik May 27, 2024
75a5b61
Merge branch 'develop' into register_target
apwojcik May 28, 2024
bbf93b0
Merge branch 'develop' into register_target
apwojcik May 29, 2024
52fc7d1
Merge branch 'develop' into register_target
apwojcik May 30, 2024
2ad56ac
Merge branch 'develop' into register_target
apwojcik Jun 3, 2024
f00dc28
Merge branch 'develop' into register_target
apwojcik Jun 5, 2024
a3ccae5
fix target library registration on Windows
apwojcik Mar 7, 2024
2004b73
add missing linking to migraphx_all_tragets
apwojcik Jun 6, 2024
14bf216
Merge branch 'develop' into register_target
apwojcik Jun 6, 2024
bcd8ad1
revert some more changes
apwojcik Jun 6, 2024
165d0ec
revert some more changes
apwojcik Jun 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions src/include/migraphx/register_target.hpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* The MIT License (MIT)
*
* Copyright (c) 2015-2023 Advanced Micro Devices, Inc. All rights reserved.
* Copyright (c) 2015-2024 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
Expand All @@ -28,12 +28,12 @@
#include <migraphx/target.hpp>
#include <migraphx/auto_register.hpp>
#include <cstring>
#include <utility>
#include <vector>

namespace migraphx {
inline namespace MIGRAPHX_INLINE_NS {

MIGRAPHX_EXPORT void register_target_init();
MIGRAPHX_EXPORT void register_target(const target& t);
MIGRAPHX_EXPORT void unregister_target(const std::string& name);
MIGRAPHX_EXPORT target make_target(const std::string& name);
Expand All @@ -44,15 +44,14 @@ struct target_handler
{
target t;
std::string target_name;
target_handler(const target& t_r) : t(t_r), target_name(t.name()) {}
explicit target_handler(target t_r) : t(std::move(t_r)), target_name(t.name()) {}
~target_handler() { unregister_target(target_name); }
};
} // namespace detail

template <class T>
void register_target()
{
register_target_init();
static auto t_h = detail::target_handler(T{});
register_target(t_h.t);
}
Expand All @@ -66,9 +65,6 @@ struct register_target_action
}
};

template <class T>
using auto_register_target = auto_register<register_target_action, T>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont remove this, We still want to provide the option to register a target with CRTP.


#define MIGRAPHX_REGISTER_TARGET(...) MIGRAPHX_AUTO_REGISTER(register_target_action, __VA_ARGS__)

} // namespace MIGRAPHX_INLINE_NS
Expand Down
24 changes: 22 additions & 2 deletions src/register_target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,28 @@
namespace migraphx {
inline namespace MIGRAPHX_INLINE_NS {

#ifdef _WIN32
namespace {
struct auto_load_targets
{
auto_load_targets()
{
make_target("ref");
#ifdef HAVE_CPU
make_target("cpu");
#endif
#ifdef HAVE_GPU
make_target("gpu");
#endif
#ifdef HAVE_FPGA
make_target("fpga");
#endif
}
};
[[maybe_unused]] static auto load_targets = auto_load_targets{};
} // namespace
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to go into a static/object library that we only link in for the tests. This prevents lazy loading the targets.

#endif

void store_target_lib(const dynamic_loader& lib)
{
static std::vector<dynamic_loader> target_loader;
Expand All @@ -44,8 +66,6 @@ std::unordered_map<std::string, target>& target_map()
return m;
}

void register_target_init() { (void)target_map(); }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont remove this, I think this was necessary to ensure targets are loaded in some cases.


void unregister_target(const std::string& name)
{
assert(target_map().count(name));
Expand Down
Loading