> -----Original Message----- > From: Georg-Johann Lay [mailto:a...@gjlay.de] > Sent: 18 June 2015 23:46 > To: Sivanupandi, Pitchumani > Cc: avr-libc-dev@nongnu.org > Subject: Re: [avr-libc-dev] [bug #43828] wdt.h: Wrong inline assembler > arguments > > Am 06/17/2015 um 12:02 PM schrieb Sivanupandi, Pitchumani: > >>> 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. > > In general it's not wanted that respective asm is moved around, and one way > of reducing the chance of motion is a memory barrier. Except in the case > where performance is absolutely crucial a generic memory clobber is fine, > IMO.
Ok. > My observation is that users are less comfortable with unexpected code > motion than with rare and minor optimization loss. > > But that's a matter of taste, of course... Joerg, what do you think about this memory clobber in wdt_enable/disable? Regards, Pitchumani _______________________________________________ AVR-libc-dev mailing list AVR-libc-dev@nongnu.org https://lists.nongnu.org/mailman/listinfo/avr-libc-dev
