All the asms are changing memory but don't mention that. Usually it
is not wanted that (non-volatile) memory accesses are dragged across
the inline asm.
Easy fix is to clobber memory, more exact is to explicitly describe
the effect on memory. For example, one sequence reads:
"sts %0, %2"
: /* no outputs */
: "M" (_SFR_MEM_ADDR(_WD_CONTROL_REG)),
Explicit memory modelling would read something like:
"sts %1, %3"
: "+m" (_WD_CONTROL_REG)
: "n" (_SFR_MEM_ADDR(_WD_CONTROL_REG)),
I am ok with your changes.
In addition, I would like to add below change (wdt_enable for
__AVR_TINY__) to avoid overwriting unintended bits.
Below are my findings in further evaluation of these changes:
Clobbering memory lead to un-optimal code when there is any memory reference
before and after wdt_enable/disable call.
Explicit memory modelling may safeguard the memory access across the inline asm.
But, even without that explicit clobber (in wdt enable/disable case, it will be
_WD_CONTROL_REG),
compiler safely reloads the content (instead of re-use the register that is
loaded already). This is
because the _WD_CONTROL_REG will be expanded as the volatile access.
For the following case, compiler may not safeguard the memory access as the WDT
address is
referred directly (without the macro, also not qualified as volatile).
*(char*)0x60 = c1;
wdt_enable(3);
c2 = *(char*)0x60;
IMHO, this usage is not common in embedded applications. So, clobbering memory
in wdt_enable/disable macros may not be required as it leads to un-optimal code.