Fix/field set side effect hazards

examples/generated/atxmega_spi_hal.h

@@ -19,6 +19,10 @@ namespace atxmega_spi_nm
     public:
         using TYPE = CTRL<BASE, WIDTH, PARENT_TYPE>;
 
+        static constexpr uint32_t onwrite_hazard_mask = 0x0;
+        static constexpr uint32_t onwrite_hazard_safe_val = 0x0;
+        static constexpr bool has_onread_hazard = false;
+
         static halcpp::FieldRW<0, 1, TYPE> PRESCALER;
         static halcpp::FieldRW<2, 3, TYPE> MODE;
         static halcpp::FieldRW<4, 4, TYPE> MASTER;
@@ -37,6 +41,10 @@ namespace atxmega_spi_nm
     public:
         using TYPE = INTCTRL<BASE, WIDTH, PARENT_TYPE>;
 
+        static constexpr uint32_t onwrite_hazard_mask = 0x0;
+        static constexpr uint32_t onwrite_hazard_safe_val = 0x0;
+        static constexpr bool has_onread_hazard = false;
+
         static halcpp::FieldRW<0, 1, TYPE> INTLVL;
 
         using halcpp::RegRW<BASE, WIDTH, PARENT_TYPE>::operator=;
@@ -50,6 +58,10 @@ namespace atxmega_spi_nm
     public:
         using TYPE = STATUS<BASE, WIDTH, PARENT_TYPE>;
 
+        static constexpr uint32_t onwrite_hazard_mask = 0x0;
+        static constexpr uint32_t onwrite_hazard_safe_val = 0x0;
+        static constexpr bool has_onread_hazard = false;
+
         static halcpp::FieldRO<6, 6, TYPE> WRCOL;
         static halcpp::FieldRO<7, 7, TYPE> IF;
     };
@@ -68,6 +80,10 @@ namespace atxmega_spi_nm
     public:
         using TYPE = DATA<BASE, WIDTH, PARENT_TYPE>;
 
+        static constexpr uint32_t onwrite_hazard_mask = 0x0;
+        static constexpr uint32_t onwrite_hazard_safe_val = 0x0;
+        static constexpr bool has_onread_hazard = false;
+
         static halcpp::FieldWO<0, 7, TYPE> WDATA;
         static halcpp::FieldRO<0, 7, TYPE> RDATA;
 

examples/generated/include/field_node.h

@@ -53,7 +53,11 @@ namespace halcpp
          */
         static constexpr uint32_t bit_mask()
         {
-            return (((1u << (width)) - 1));
+            if constexpr (width >= 32) {
+                return ~0u;
+            } else {
+                return ((1u << width) - 1u);
+            }
         }
 
 
@@ -95,7 +99,20 @@ namespace halcpp
         static inline void set(typename BASE_TYPE::dataType val)
         {
             if constexpr (node_has_get_v<parent>)
-                parent::set((parent::get() & BASE_TYPE::field_mask()) | ((val & BASE_TYPE::bit_mask()) << BASE_TYPE::start_bit));
+            {
+                static_assert(!node_onread_hazard_v<parent>,
+                    "This register contains a field with a destructive-read side effect "
+                    "(onread=rclr/rset/ruser). Setting a single field requires a "
+                    "read-modify-write, which would silently disturb that field. Write the "
+                    "whole register instead (e.g. `my_reg = value;`).");
+
+                uint32_t reg_val = parent::get();
+                if constexpr (node_has_onwrite_hazard_v<parent>)
+                    reg_val = (reg_val & ~parent::onwrite_hazard_mask)
+                            | (parent::onwrite_hazard_safe_val & parent::onwrite_hazard_mask);
+
+                parent::set((reg_val & BASE_TYPE::field_mask()) | ((val & BASE_TYPE::bit_mask()) << BASE_TYPE::start_bit));
+            }
             else
                 parent::set(val << BASE_TYPE::start_bit);
         }

examples/generated/include/halcpp_utils.h

@@ -127,6 +127,73 @@ namespace halcpp
     template <class TYPE>
     constexpr bool node_has_get_v = node_has_get<TYPE>::value;
 
+    /**
+     * @brief Type trait to check if a type has an `onwrite_hazard_mask` static member.
+     *
+     * Used to detect (at compile time) whether a register type carries the write-side-effect
+     * masking constants emitted by the code generator, without requiring every possible
+     * parent type to define them.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE, class = void>
+    struct node_has_onwrite_hazard : std::false_type
+    {
+    };
+
+    /**
+     * @brief Specialization of `node_has_onwrite_hazard` for types that define
+     * `onwrite_hazard_mask`.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE>
+    struct node_has_onwrite_hazard<TYPE, std::void_t<decltype(TYPE::onwrite_hazard_mask)>> : std::true_type
+    {
+    };
+
+    /**
+     * @brief Convenience variable template for `node_has_onwrite_hazard`.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE>
+    constexpr bool node_has_onwrite_hazard_v = node_has_onwrite_hazard<TYPE>::value;
+
+    /**
+     * @brief Type trait exposing a register's `has_onread_hazard` flag, defaulting to false
+     * for types that don't define it.
+     *
+     * A register has an onread hazard when one of its fields has a destructive-read side
+     * effect (onread=rclr/rset/ruser). Reading such a register - even incidentally, as part
+     * of a read-modify-write to set a sibling field - silently disturbs it.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE, class = void>
+    struct node_onread_hazard : std::false_type
+    {
+    };
+
+    /**
+     * @brief Specialization of `node_onread_hazard` for types that define `has_onread_hazard`.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE>
+    struct node_onread_hazard<TYPE, std::void_t<decltype(TYPE::has_onread_hazard)>>
+        : std::integral_constant<bool, TYPE::has_onread_hazard>
+    {
+    };
+
+    /**
+     * @brief Convenience variable template for `node_onread_hazard`.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE>
+    constexpr bool node_onread_hazard_v = node_onread_hazard<TYPE>::value;
+
 }
 
 #endif // !_HALCPP_UTILS_H_

examples/generated/regs_and_mem_hal.h

@@ -19,6 +19,10 @@ namespace regs_and_mem_nm
     public:
         using TYPE = CSR<BASE, WIDTH, PARENT_TYPE>;
 
+        static constexpr uint32_t onwrite_hazard_mask = 0x0;
+        static constexpr uint32_t onwrite_hazard_safe_val = 0x0;
+        static constexpr bool has_onread_hazard = false;
+
         static halcpp::FieldRW<0, 7, TYPE> ctrl;
 
         using halcpp::RegRW<BASE, WIDTH, PARENT_TYPE>::operator=;
@@ -32,6 +36,10 @@ namespace regs_and_mem_nm
     public:
         using TYPE = CSR2<BASE, WIDTH, PARENT_TYPE>;
 
+        static constexpr uint32_t onwrite_hazard_mask = 0x0;
+        static constexpr uint32_t onwrite_hazard_safe_val = 0x0;
+        static constexpr bool has_onread_hazard = false;
+
         static halcpp::FieldRW<0, 7, TYPE> ctrl;
 
         using halcpp::RegRW<BASE, WIDTH, PARENT_TYPE>::operator=;

src/peakrdl_halcpp/halnode.py

@@ -1,4 +1,4 @@
-from typing import TYPE_CHECKING, Optional, Iterator, List
+from typing import TYPE_CHECKING, Optional, Iterator, List, Tuple
 import itertools
 import logging
 
@@ -302,6 +302,18 @@ def cpp_access_type(self) -> str:
             raise ValueError(f'Node field access rights are not found \
                               {self.inst.inst_name}')
 
+    @property
+    def onwrite(self) -> Optional[str]:
+        """Returns the SystemRDL 'onwrite' property name (e.g. 'woclr'), or None if unset."""
+        onwrite_prop = self.get_property('onwrite')
+        return onwrite_prop.name if onwrite_prop is not None else None
+
+    @property
+    def onread(self) -> Optional[str]:
+        """Returns the SystemRDL 'onread' property name (e.g. 'rclr'), or None if unset."""
+        onread_prop = self.get_property('onread')
+        return onread_prop.name if onread_prop is not None else None
+
     def get_enums(self):
         """Returns the enumeration(s) of a FieldNode.
 
@@ -379,6 +391,54 @@ def width(self) -> int:
         """Returns the register width in bits, derived from the highest bit position of its fields."""
         return max([c.high for c in self.halchildren(HalFieldNode)]) + 1
 
+    # onwrite values where writing 0 is always a no-op (safe to echo back 0)
+    _ONWRITE_ZERO_SAFE = {'woset', 'woclr', 'wot'}
+    # onwrite values where writing 1 is always a no-op (safe to echo back 1)
+    _ONWRITE_ONE_SAFE = {'wzs', 'wzc', 'wzt'}
+    # onwrite values that trigger unconditionally on any write; no safe echo value exists
+    _ONWRITE_UNSAFE = {'wclr', 'wset', 'wuser'}
+
+    def get_onwrite_hazard(self) -> Tuple[int, int]:
+        """Computes the bitmask/safe-echo-value needed to avoid disturbing sibling fields
+        with write side effects (onwrite=woclr/woset/... etc.) when another field in the
+        same register is set via read-modify-write.
+
+        Returns
+        -------
+        tuple
+            ``(mask, safe_val)`` where ``mask`` has a bit set for every bit position
+            belonging to a field with a maskable onwrite side effect, and ``safe_val``
+            holds the no-op bit value (0 or 1) to echo back for each of those bits.
+        """
+        mask = 0
+        safe_val = 0
+        for f in self.halchildren(HalFieldNode):
+            onwrite = f.onwrite
+            if onwrite is None:
+                continue
+            field_bits = ((1 << (f.high - f.low + 1)) - 1) << f.low
+            if onwrite in HalRegNode._ONWRITE_UNSAFE:
+                halnode_logger.warning(
+                    f"Field '{f.inst_name}' in register '{self.inst_name}' has onwrite={onwrite}, "
+                    f"which triggers unconditionally on any write to the register. This cannot be "
+                    f"protected against in software; writing any other field in this register will "
+                    f"disturb '{f.inst_name}'.")
+                continue
+            mask |= field_bits
+            if onwrite in HalRegNode._ONWRITE_ONE_SAFE:
+                safe_val |= field_bits
+        return mask, safe_val
+
+    def has_onread_hazard(self) -> bool:
+        """Returns True if any field in this register has an onread side effect
+        (e.g. rclr, rset), making a full register read destructive.
+
+        When True, per-field set() (which performs a read-modify-write) cannot be used
+        safely on this register: a mere incidental read of the register (needed to
+        preserve the other fields' values) would silently disturb this field.
+        """
+        return any(f.onread is not None for f in self.halchildren(HalFieldNode))
+
     def get_template_line(self) -> str:
         """Returns the class template string."""
         return f"template <uint32_t BASE, uint32_t WIDTH, typename PARENT_TYPE>"

src/peakrdl_halcpp/include/field_node.h

@@ -99,7 +99,20 @@ namespace halcpp
         static inline void set(typename BASE_TYPE::dataType val)
         {
             if constexpr (node_has_get_v<parent>)
-                parent::set((parent::get() & BASE_TYPE::field_mask()) | ((val & BASE_TYPE::bit_mask()) << BASE_TYPE::start_bit));
+            {
+                static_assert(!node_onread_hazard_v<parent>,
+                    "This register contains a field with a destructive-read side effect "
+                    "(onread=rclr/rset/ruser). Setting a single field requires a "
+                    "read-modify-write, which would silently disturb that field. Write the "
+                    "whole register instead.");
+
+                uint32_t reg_val = parent::get();
+                if constexpr (node_has_onwrite_hazard_v<parent>)
+                    reg_val = (reg_val & ~parent::onwrite_hazard_mask)
+                            | (parent::onwrite_hazard_safe_val & parent::onwrite_hazard_mask);
+
+                parent::set((reg_val & BASE_TYPE::field_mask()) | ((val & BASE_TYPE::bit_mask()) << BASE_TYPE::start_bit));
+            }
             else
                 parent::set(val << BASE_TYPE::start_bit);
         }

src/peakrdl_halcpp/include/halcpp_utils.h

@@ -127,6 +127,73 @@ namespace halcpp
     template <class TYPE>
     constexpr bool node_has_get_v = node_has_get<TYPE>::value;
 
+    /**
+     * @brief Type trait to check if a type has an `onwrite_hazard_mask` static member.
+     *
+     * Used to detect (at compile time) whether a register type carries the write-side-effect
+     * masking constants emitted by the code generator, without requiring every possible
+     * parent type to define them.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE, class = void>
+    struct node_has_onwrite_hazard : std::false_type
+    {
+    };
+
+    /**
+     * @brief Specialization of `node_has_onwrite_hazard` for types that define
+     * `onwrite_hazard_mask`.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE>
+    struct node_has_onwrite_hazard<TYPE, std::void_t<decltype(TYPE::onwrite_hazard_mask)>> : std::true_type
+    {
+    };
+
+    /**
+     * @brief Convenience variable template for `node_has_onwrite_hazard`.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE>
+    constexpr bool node_has_onwrite_hazard_v = node_has_onwrite_hazard<TYPE>::value;
+
+    /**
+     * @brief Type trait exposing a register's `has_onread_hazard` flag, defaulting to false
+     * for types that don't define it.
+     *
+     * A register has an onread hazard when one of its fields has a destructive-read side
+     * effect (onread=rclr/rset/ruser). Reading such a register - even incidentally, as part
+     * of a read-modify-write to set a sibling field - silently disturbs it.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE, class = void>
+    struct node_onread_hazard : std::false_type
+    {
+    };
+
+    /**
+     * @brief Specialization of `node_onread_hazard` for types that define `has_onread_hazard`.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE>
+    struct node_onread_hazard<TYPE, std::void_t<decltype(TYPE::has_onread_hazard)>>
+        : std::integral_constant<bool, TYPE::has_onread_hazard>
+    {
+    };
+
+    /**
+     * @brief Convenience variable template for `node_onread_hazard`.
+     *
+     * @tparam TYPE The type to check.
+     */
+    template <class TYPE>
+    constexpr bool node_onread_hazard_v = node_onread_hazard<TYPE>::value;
+
 }
 
 #endif // !_HALCPP_UTILS_H_

src/peakrdl_halcpp/templates/addrmap.h.j2

@@ -51,6 +51,11 @@ namespace {{ halnode.orig_type_name }}_nm
     public:
         using TYPE = {{ r.orig_type_name|upper }}{{ r.get_cls_tmpl_params() }};
 
+    {% set hz_mask, hz_safe_val = r.get_onwrite_hazard() %}
+        static constexpr uint32_t onwrite_hazard_mask = 0x{{ "%0x"|format(hz_mask) }};
+        static constexpr uint32_t onwrite_hazard_safe_val = 0x{{ "%0x"|format(hz_safe_val) }};
+        static constexpr bool has_onread_hazard = {{ "true" if r.has_onread_hazard() else "false" }};
+
     {# Add the fields to the register #}
     {% for f in r.halchildren(children_type=HalFieldNode, skip_buses=skip_buses) %}
         static halcpp::{{ f.cpp_access_type }}<{{ f.low }}, {{ f.high }}, TYPE> {{ f.inst_name }};
@@ -91,6 +96,11 @@ namespace {{ halnode.orig_type_name }}_nm
     public:
         using TYPE = {{ r.orig_type_name|upper }}{{ r.get_cls_tmpl_params() }};
 
+    {% set hz_mask, hz_safe_val = r.get_onwrite_hazard() %}
+        static constexpr uint32_t onwrite_hazard_mask = 0x{{ "%0x"|format(hz_mask) }};
+        static constexpr uint32_t onwrite_hazard_safe_val = 0x{{ "%0x"|format(hz_safe_val) }};
+        static constexpr bool has_onread_hazard = {{ "true" if r.has_onread_hazard() else "false" }};
+
     {# Add the fields to the register #}
     {% for f in r.halchildren(children_type=HalFieldNode, skip_buses=skip_buses) %}
         static halcpp::{{ f.cpp_access_type }}<{{ f.low }}, {{ f.high }}, TYPE> {{ f.inst_name }};