Skip to content

Commit

Permalink
Cleanup clangtidy warnings (#8057)
Browse files Browse the repository at this point in the history
Relatively benign fixes to core/common/api to satisyfy clang-tidy.

Signed-off-by: Soren Soe <[email protected]>
  • Loading branch information
stsoe authored Apr 9, 2024
1 parent ff3837c commit 58cba71
Show file tree
Hide file tree
Showing 22 changed files with 275 additions and 223 deletions.
25 changes: 19 additions & 6 deletions src/runtime_src/core/common/api/aie/xrt_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ class graph_impl
device->close_graph(handle);
}

xclGraphHandle
graph_impl() = delete;
graph_impl(const graph_impl&) = delete;
graph_impl(graph_impl&&) = delete;
graph_impl& operator=(const graph_impl&) = delete;
graph_impl& operator=(graph_impl&&) = delete;

[[nodiscard]] xclGraphHandle
get_handle() const
{
return handle;
Expand Down Expand Up @@ -112,7 +118,7 @@ class graph_impl

}

namespace xrt { namespace aie {
namespace xrt::aie {

class profiling_impl
{
Expand All @@ -124,7 +130,7 @@ class profiling_impl
using handle = int;
static constexpr handle invalid_handle = -1;

profiling_impl(std::shared_ptr<xrt_core::device> dev)
explicit profiling_impl(std::shared_ptr<xrt_core::device> dev)
: device(std::move(dev)),
profiling_hdl(invalid_handle)
{}
Expand All @@ -139,6 +145,13 @@ class profiling_impl
}
}

profiling_impl() = delete;
profiling_impl(const profiling_impl&) = delete;
profiling_impl(profiling_impl&&) = delete;
profiling_impl& operator=(const profiling_impl&) = delete;
profiling_impl& operator=(profiling_impl&&) = delete;


handle
start_profiling(int option, const std::string& port1_name, const std::string& port2_name, uint32_t value)
{
Expand Down Expand Up @@ -168,7 +181,7 @@ class profiling_impl

};

}}
} // xrt::aie

namespace {

Expand Down Expand Up @@ -380,7 +393,7 @@ read_port(const std::string& port_name, void* value, size_t bytes)
////////////////////////////////////////////////////////////////
// xrt_aie_profiling C++ API implmentations (xrt_aie.h)
////////////////////////////////////////////////////////////////
namespace xrt { namespace aie {
namespace xrt::aie {

profiling::
profiling(const xrt::device& device)
Expand Down Expand Up @@ -415,7 +428,7 @@ stop() const
});
}

}} //namespace aie, xrt
} // xrt:aie

////////////////////////////////////////////////////////////////
// xrt_aie API implementations (xrt_aie.h, xrt_graph.h)
Expand Down
20 changes: 8 additions & 12 deletions src/runtime_src/core/common/api/context_mgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

using namespace std::chrono_literals;

namespace xrt_core { namespace context_mgr {
namespace xrt_core::context_mgr {

// class device_context_mgr - synchroize open and close context for IPs
//
Expand All @@ -41,8 +41,8 @@ class device_context_mgr
std::string ipname;
cuidx_type ipidx;

ip(const std::string& nm, cuidx_type idx)
: ipname(nm), ipidx(idx)
ip(std::string nm, cuidx_type idx)
: ipname(std::move(nm)), ipidx(idx)
{}
};

Expand Down Expand Up @@ -79,13 +79,8 @@ class device_context_mgr
std::mutex m_mutex;
std::map<const hwctx_handle*, ctx> m_ctx;
std::condition_variable m_cv;
xrt_core::device* m_device;

public:
device_context_mgr(xrt_core::device* device)
: m_device(device)
{}

// Open context on IP in specified hardware context.
// Open the IP context when it is safe to do so. Note, that usage
// of the context manager does not support multiple threads calling
Expand Down Expand Up @@ -127,14 +122,15 @@ class device_context_mgr
// Get (and create) context manager for device.
// Cache the created manager so other threads can share.
static std::shared_ptr<device_context_mgr>
get_device_context_mgr(xrt_core::device* device, bool create = false)
get_device_context_mgr(const xrt_core::device* device, bool create = false)
{
static std::map<const xrt_core::device*, std::weak_ptr<device_context_mgr>> d2cmgr;
static std::mutex ctx_mutex;
std::lock_guard<std::mutex> lk(ctx_mutex);
auto cmgr = d2cmgr[device].lock();
if (!cmgr && create)
d2cmgr[device] = cmgr = std::shared_ptr<device_context_mgr>(new device_context_mgr(device));
// NOLINTNEXTLINE using new for weak_ptr
d2cmgr[device] = cmgr = std::shared_ptr<device_context_mgr>(new device_context_mgr);
return cmgr;
}

Expand All @@ -147,7 +143,7 @@ create(const xrt_core::device* device)
{
// creating a context manager doesn't change device, but aqcuiring a
// context is a device operation and cannot use const device
return get_device_context_mgr(const_cast<xrt_core::device*>(device), true);
return get_device_context_mgr(device, true);
}

// Regular CU
Expand All @@ -173,4 +169,4 @@ close_context(const xrt::hw_context& hwctx, cuidx_type cuidx)
throw std::runtime_error("No context manager for device");
}

}} // context_mgr, xrt_core
} // xrt_core::context_mgr
39 changes: 26 additions & 13 deletions src/runtime_src/core/common/api/hw_queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ class command_manager
std::mutex work_mutex;
std::condition_variable work_cond;
command_queue_type submitted_cmds;
uint64_t exec_wait_call_count = 0;
bool stop = false;

// thread can be constructed only after data members are initialized
Expand All @@ -126,7 +125,7 @@ class command_manager
std::vector<xrt_core::command*> busy_cmds;
std::vector<xrt_core::command*> running_cmds;

while (1) {
while (true) {

// Larger wait synchronized with launch()
{
Expand Down Expand Up @@ -197,7 +196,7 @@ class command_manager

public:
// Constructor starts monitor thread
command_manager(executor* impl)
explicit command_manager(executor* impl)
: m_impl(impl), monitor_thread(xrt_core::thread(&command_manager::monitor, this))
{
XRT_DEBUGF("command_manager::command_manager(0x%x)\n", impl);
Expand All @@ -218,6 +217,12 @@ class command_manager
monitor_thread.join();
}

command_manager() = delete;
command_manager(const command_manager&) = delete;
command_manager(command_manager&&) = delete;
command_manager& operator=(const command_manager&) = delete;
command_manager& operator=(command_manager&&) = delete;

void
clear_executor()
{
Expand Down Expand Up @@ -349,13 +354,18 @@ class hw_queue_impl : public command_manager::executor
}
}

hw_queue_impl(const hw_queue_impl&) = delete;
hw_queue_impl(hw_queue_impl&&) = delete;
hw_queue_impl& operator=(const hw_queue_impl&) = delete;
hw_queue_impl& operator=(hw_queue_impl&&) = delete;

// Submit command for execution
virtual void
submit(xrt_core::command* cmd) = 0;
submit(xrt_core::command* cmd) = 0; // NOLINT override from base

// Wait for some command to finish
virtual std::cv_status
wait(size_t timeout_ms) = 0;
wait(size_t timeout_ms) = 0; // NOLINT override from base

// Wait for specified command to finish
virtual std::cv_status
Expand Down Expand Up @@ -428,7 +438,7 @@ class qds_device : public hw_queue_impl
// notify_host is not strictly necessary for unmanaged
// command execution but provides a central place to update
// and mark commands as done so they can be re-executed.
notify_host(const_cast<xrt_core::command*>(cmd), static_cast<ert_cmd_state>(pkt->state));
notify_host(const_cast<xrt_core::command*>(cmd), static_cast<ert_cmd_state>(pkt->state)); // NOLINT

return std::cv_status::no_timeout;
}
Expand Down Expand Up @@ -514,18 +524,20 @@ class kds_device : public hw_queue_impl
// Some other thread is calling device::exec_wait, wait
// for it complete its work and notify this thread
auto status = std::cv_status::no_timeout;
if (timeout_ms)
if (timeout_ms) {
status = (m_work.wait_for(lk, timeout_ms * 1ms,
[this] {
return thread_exec_wait_call_count != m_exec_wait_call_count;
}))
? std::cv_status::no_timeout
: std::cv_status::timeout;
else
}
else {
m_work.wait(lk,
[this] {
return thread_exec_wait_call_count != m_exec_wait_call_count;
});
}

// The other thread has completed its exec_wait call,
// sync with current global call count and return
Expand Down Expand Up @@ -555,7 +567,8 @@ class kds_device : public hw_queue_impl
}
else {
// wait for ever for some command to complete
while (m_device->exec_wait(1000) == 0) {}
constexpr size_t default_timeout = 1000;
while (m_device->exec_wait(default_timeout) == 0) {}
}

// Acquire lock before updating shared state
Expand All @@ -573,7 +586,7 @@ class kds_device : public hw_queue_impl
}

public:
kds_device(xrt_core::device* device)
explicit kds_device(xrt_core::device* device)
: m_device(device)
{}

Expand All @@ -596,7 +609,7 @@ class kds_device : public hw_queue_impl
// notify_host is not strictly necessary for unmanaged
// command execution but provides a central place to update
// and mark commands as done so they can be re-executed.
notify_host(const_cast<xrt_core::command*>(cmd), static_cast<ert_cmd_state>(pkt->state));
notify_host(const_cast<xrt_core::command*>(cmd), static_cast<ert_cmd_state>(pkt->state)); // NOLINT

return std::cv_status::no_timeout;
}
Expand Down Expand Up @@ -651,7 +664,7 @@ exiting()
return lights_out;
}

struct uninit
struct uninit // NOLINT
{
~uninit() { exiting() = true; }
};
Expand All @@ -668,7 +681,7 @@ get_kds_device_nolock(hwc2hwq_type& queues, const xrt_core::device* device)
auto hwqimpl = queues[nullptr].lock();
if (!hwqimpl)
queues[nullptr] = hwqimpl =
queue_ptr{new xrt_core::kds_device(const_cast<xrt_core::device*>(device))};
queue_ptr{new xrt_core::kds_device(const_cast<xrt_core::device*>(device))}; // NOLINT

return hwqimpl;
}
Expand Down
4 changes: 2 additions & 2 deletions src/runtime_src/core/common/api/native_profile.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ profiling_wrapper(const char* function, Callable&& f, Args&&...args)
if (xrt_core::config::get_native_xrt_trace()
|| xrt_core::config::get_host_trace()) {
generic_api_call_logger log_object(function) ;
return f(std::forward<Args>(args)...) ;
return f(std::forward<Args>(args)...) ; // NOLINT, clang-tidy false positive [potential leak]
}
return f(std::forward<Args>(args)...) ;
return f(std::forward<Args>(args)...) ; // NOLINT, clang-tidy false positive [potential leak]
}

// Specializations of the logger for capturing different information
Expand Down
Loading

0 comments on commit 58cba71

Please sign in to comment.