From 5cf8a851d879d96790d26dadd9cc5ca2af38874c Mon Sep 17 00:00:00 2001 From: Nicolas Clauvelin Date: Thu, 22 Feb 2018 10:22:51 -0500 Subject: [PATCH] FIX INLINING OF WRITE CALLS WITH Os ENABLED The inlining issue was due to a bad template design in the access policies implementations: the MMIO_t type from the field was used (which is a volatile type) for all arguments in the write implementation. The new implementation relies on two template types (one for the register memory device and another one for the value to be written). This fixes inlining and was checked on godbolt. --- policies/AccessPolicy.h | 34 +++++++++++++--------- register/Field.h | 34 +++++++++------------- single/cppreg-all.h | 63 ++++++++++++++++++++--------------------- 3 files changed, 65 insertions(+), 66 deletions(-) diff --git a/policies/AccessPolicy.h b/policies/AccessPolicy.h index 6738834..b3c20cd 100644 --- a/policies/AccessPolicy.h +++ b/policies/AccessPolicy.h @@ -25,14 +25,15 @@ namespace cppreg { //! Read access implementation. /** + * @tparam MMIO_t Register memory device type. * @tparam T Field data type. * @param mmio_device Pointer to register mapped memory. * @param mask Field mask. * @param offset Field offset. * @return The value at the field location. */ - template - inline static T read(const T* const mmio_device, + template + inline static T read(const MMIO_t* const mmio_device, const T mask, const Offset_t offset) noexcept { return static_cast((*mmio_device & mask) >> offset); @@ -46,14 +47,15 @@ namespace cppreg { //! Write access implementation. /** + * @tparam MMIO_t Register memory device type. * @tparam T Field data type. * @param mmio_device Pointer to register mapped memory. * @param mask Field mask. * @param offset Field offset. * @param value Value to be written at the field location. */ - template - inline static void write(T* const mmio_device, + template + inline static void write(MMIO_t* const mmio_device, const T mask, const Offset_t offset, const T value) noexcept { @@ -67,31 +69,36 @@ namespace cppreg { * @param mmio_device Pointer to register mapped memory. * @param mask Field mask. */ - template - inline static void set(T* const mmio_device, const T mask) noexcept { + template + inline static void set(MMIO_t* const mmio_device, const T mask) + noexcept { *mmio_device = static_cast((*mmio_device) | mask); }; //! Clear field implementation. /** + * @tparam MMIO_t Register memory device type. * @tparam T Field data type. * @param mmio_device Pointer to register mapped memory. * @param mask Field mask. */ - template - inline static void clear(T* const mmio_device, const T mask) noexcept { + template + inline static void clear(MMIO_t* const mmio_device, const T mask) + noexcept { *mmio_device = static_cast((*mmio_device) & ~mask); }; //! Toggle field implementation. /** + * @tparam MMIO_t Register memory device type. * @tparam T Field data type. * @param mmio_device Pointer to register mapped memory. * @param mask Field mask. */ - template - inline static void toggle(T* const mmio_device, const T mask) noexcept { - *mmio_device ^= mask; + template + inline static void toggle(MMIO_t* const mmio_device, const T mask) + noexcept { + *mmio_device = static_cast((*mmio_device) ^ mask); }; }; @@ -102,14 +109,15 @@ namespace cppreg { //! Write access implementation. /** + * @tparam MMIO_t Register memory device type. * @tparam T Field data type. * @param mmio_device Pointer to register mapped memory. * @param mask Field mask. * @param offset Field offset * @param value Value to be written at the field location. */ - template - inline static void write(T* const mmio_device, + template + inline static void write(MMIO_t* const mmio_device, const T mask, const Offset_t offset, const T value) noexcept { diff --git a/register/Field.h b/register/Field.h index e4ffacf..86874c7 100644 --- a/register/Field.h +++ b/register/Field.h @@ -90,10 +90,9 @@ namespace cppreg { */ inline static type read() noexcept { return - AccessPolicy - ::template read(parent_register::ro_mem_pointer(), - mask, - offset); + AccessPolicy::read(parent_register::ro_mem_pointer(), + mask, + offset); }; //! Field write method (shadow value disabled). @@ -113,11 +112,10 @@ namespace cppreg { write(const typename std::enable_if< !parent_register::shadow::use_shadow, T >::type value) noexcept { - AccessPolicy - ::template write(parent_register::rw_mem_pointer(), - mask, - offset, - value); + AccessPolicy::write(parent_register::rw_mem_pointer(), + mask, + offset, + value); }; //! Field write method (shadow value enabled). @@ -147,11 +145,10 @@ namespace cppreg { // Write as a block to the register, that is, we do not use the // mask and offset. - AccessPolicy - ::template write(parent_register::rw_mem_pointer(), - ~(0u), - 0u, - parent_register::shadow::value); + AccessPolicy::write(parent_register::rw_mem_pointer(), + ~(0u), + 0u, + parent_register::shadow::value); }; @@ -205,8 +202,7 @@ namespace cppreg { * This method will set all bits in the field. */ inline static void set() noexcept { - AccessPolicy - ::template set(parent_register::rw_mem_pointer(), mask); + AccessPolicy::set(parent_register::rw_mem_pointer(), mask); }; //! Field clear method. @@ -214,8 +210,7 @@ namespace cppreg { * This method will clear all bits in the field. */ inline static void clear() noexcept { - AccessPolicy - ::template clear(parent_register::rw_mem_pointer(), mask); + AccessPolicy::clear(parent_register::rw_mem_pointer(), mask); }; //! Field toggle method. @@ -223,8 +218,7 @@ namespace cppreg { * This method will toggle all bits in the field. */ inline static void toggle() noexcept { - AccessPolicy - ::template toggle(parent_register::rw_mem_pointer(), mask); + AccessPolicy::toggle(parent_register::rw_mem_pointer(), mask); }; //! Is field set bool method. diff --git a/single/cppreg-all.h b/single/cppreg-all.h index dd20ad6..b05ebcc 100644 --- a/single/cppreg-all.h +++ b/single/cppreg-all.h @@ -24,38 +24,41 @@ namespace cppreg { #define CPPREG_ACCESSPOLICY_H namespace cppreg { struct read_only { - template - inline static T read(const T* const mmio_device, + template + inline static T read(const MMIO_t* const mmio_device, const T mask, const Offset_t offset) noexcept { return static_cast((*mmio_device & mask) >> offset); }; }; struct read_write : read_only { - template - inline static void write(T* const mmio_device, + template + inline static void write(MMIO_t* const mmio_device, const T mask, const Offset_t offset, const T value) noexcept { *mmio_device = static_cast((*mmio_device & ~mask) | ((value << offset) & mask)); }; - template - inline static void set(T* const mmio_device, const T mask) noexcept { + template + inline static void set(MMIO_t* const mmio_device, const T mask) + noexcept { *mmio_device = static_cast((*mmio_device) | mask); }; - template - inline static void clear(T* const mmio_device, const T mask) noexcept { + template + inline static void clear(MMIO_t* const mmio_device, const T mask) + noexcept { *mmio_device = static_cast((*mmio_device) & ~mask); }; - template - inline static void toggle(T* const mmio_device, const T mask) noexcept { - *mmio_device ^= mask; + template + inline static void toggle(MMIO_t* const mmio_device, const T mask) + noexcept { + *mmio_device = static_cast((*mmio_device) ^ mask); }; }; struct write_only { - template - inline static void write(T* const mmio_device, + template + inline static void write(MMIO_t* const mmio_device, const T mask, const Offset_t offset, const T value) noexcept { @@ -287,21 +290,19 @@ namespace cppreg { }; inline static type read() noexcept { return - AccessPolicy - ::template read(parent_register::ro_mem_pointer(), - mask, - offset); + AccessPolicy::read(parent_register::ro_mem_pointer(), + mask, + offset); }; template inline static void write(const typename std::enable_if< !parent_register::shadow::use_shadow, T >::type value) noexcept { - AccessPolicy - ::template write(parent_register::rw_mem_pointer(), - mask, - offset, - value); + AccessPolicy::write(parent_register::rw_mem_pointer(), + mask, + offset, + value); }; template inline static void @@ -311,11 +312,10 @@ namespace cppreg { parent_register::shadow::value = (parent_register::shadow::value & ~mask) | ((value << offset) & mask); - AccessPolicy - ::template write(parent_register::rw_mem_pointer(), - ~(0u), - 0u, - parent_register::shadow::value); + AccessPolicy::write(parent_register::rw_mem_pointer(), + ~(0u), + 0u, + parent_register::shadow::value); }; template inline static @@ -340,16 +340,13 @@ namespace cppreg { write(value); }; inline static void set() noexcept { - AccessPolicy - ::template set(parent_register::rw_mem_pointer(), mask); + AccessPolicy::set(parent_register::rw_mem_pointer(), mask); }; inline static void clear() noexcept { - AccessPolicy - ::template clear(parent_register::rw_mem_pointer(), mask); + AccessPolicy::clear(parent_register::rw_mem_pointer(), mask); }; inline static void toggle() noexcept { - AccessPolicy - ::template toggle(parent_register::rw_mem_pointer(), mask); + AccessPolicy::toggle(parent_register::rw_mem_pointer(), mask); }; template inline static typename std::enable_if::type