Skip to content

Force operator< to be inlined to reduce place time #1265

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

Merged
merged 1 commit into from
Apr 13, 2020
Merged
Changes from all commits
Commits
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
36 changes: 33 additions & 3 deletions vpr/src/place/place_delay_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@
#include "vpr_types.h"
#include "router_delay_profiling.h"

#ifndef __has_attribute
# define __has_attribute(x) 0 // Compatibility with non-clang compilers.
#endif

#if defined(COMPILER_GCC) && defined(NDEBUG)
# define ALWAYS_INLINE inline __attribute__((__always_inline__))
#elif defined(COMPILER_MSVC) && defined(NDEBUG)
# define ALWAYS_INLINE __forceinline
#elif __has_attribute(always_inline)
# define ALWAYS_INLINE __attribute__((always_inline)) // clang
#else
# define ALWAYS_INLINE inline
#endif

Comment on lines +9 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would be better to use C++ attributes (available since C++11) as a cleaner way to specify this.

For example:

[[gnu::always_inline]] friend bool operator<(const t_override& lhs, const t_override& rhs) {

That would avoid having to define the ALWAYS_INLINE macro in a cleaner cross-compiler way.

@mithro Any thoughts on this?

//Abstract interface to a placement delay model
class PlaceDelayModel {
public:
Expand Down Expand Up @@ -95,13 +109,29 @@ class OverrideDelayModel : public PlaceDelayModel {
short delta_x;
short delta_y;

friend bool operator<(const t_override& lhs, const t_override& rhs) {
return std::tie(lhs.from_type, lhs.to_type, lhs.from_class, lhs.to_class, lhs.delta_x, lhs.delta_y)
< std::tie(rhs.from_type, rhs.to_type, rhs.from_class, rhs.to_class, rhs.delta_x, rhs.delta_y);
//A combination of ALWAYS_INLINE attribute and std::lexicographical_compare
//is required for operator< to be inlined by compiler.
//Proper inlining of the function reduces place time by around 5%.
//For more information: https://github.com/verilog-to-routing/vtr-verilog-to-routing/issues/1225
friend ALWAYS_INLINE bool operator<(const t_override& lhs, const t_override& rhs) {
const short* left = reinterpret_cast<const short*>(&lhs);
const short* right = reinterpret_cast<const short*>(&rhs);
constexpr size_t NUM_T_OVERRIDE_MEMBERS = sizeof(t_override) / sizeof(short);
return std::lexicographical_compare(left, left + NUM_T_OVERRIDE_MEMBERS, right, right + NUM_T_OVERRIDE_MEMBERS);
}
};

vtr::flat_map2<t_override, float> delay_overrides_;

//operator< treats memory layout of t_override as an array of short
//this requires all members of t_override are shorts and there is no padding between members of t_override
static_assert(sizeof(t_override) == sizeof(t_override::from_type) + sizeof(t_override::to_type) + sizeof(t_override::from_class) + sizeof(t_override::to_class) + sizeof(t_override::delta_x) + sizeof(t_override::delta_y), "Expect t_override to have a memory layout equivalent to an array of short (no padding)");
static_assert(sizeof(t_override::from_type) == sizeof(short), "Expect all t_override data members to be shorts");
static_assert(sizeof(t_override::to_type) == sizeof(short), "Expect all t_override data members to be shorts");
static_assert(sizeof(t_override::from_class) == sizeof(short), "Expect all t_override data members to be shorts");
static_assert(sizeof(t_override::to_class) == sizeof(short), "Expect all t_override data members to be shorts");
static_assert(sizeof(t_override::delta_x) == sizeof(short), "Expect all t_override data members to be shorts");
static_assert(sizeof(t_override::delta_y) == sizeof(short), "Expect all t_override data members to be shorts");
};

#endif