On 15.04.21 08:46, Rasmus Villemoes wrote:
On 15/04/2021 07.34, Stefan Roese wrote:
On 13.04.21 21:38, Rasmus Villemoes wrote:
I have checked with objdump that the generated code doesn't change when this option is left at its default value of 0: gcc is smart enough to see that wd_limit is constant 0, the ">=" comparisons are tautologically true, hence all assignments to "flushed" are eliminated
Nitpicking: s/tautologically/autologically.
No.
https://www.merriam-webster.com/dictionary/tautological https://en.wikipedia.org/wiki/Tautology_(logic) "In Mathematical logic, a tautology (from Greek: ταυτολογία) is a formula or assertion that is true in every possible interpretation."
It even exists in gcc docs in the form of -Wtautological-compare. [But I do admit that "tautologically true" is a pleonasm; "tautologies" or "automatically true" could have been used instead. And going full meta, now that I look up pleonasm in wikipedia to check that I'm using it right, that article has a reference to "tautology (language)" in its second sentence.]
Interesting. Thanks for the detailed info.
/* wait for all dcbst to complete on bus */ asm volatile("sync" : : : "memory"); @@ -27,7 +33,11 @@ void flush_cache(ulong start_addr, ulong size) for (addr = start; (addr <= end) && (addr >= start); addr += CONFIG_SYS_CACHELINE_SIZE) { asm volatile("icbi 0,%0" : : "r" (addr) : "memory"); - WATCHDOG_RESET(); + flushed += CONFIG_SYS_CACHELINE_SIZE; + if (flushed >= wd_limit) { + WATCHDOG_RESET(); + flushed = 0; + }
It might make sense to pull these 2 identical code snippets into a function to not duplicate this code.
Something like
static ulong maybe_reset(ulong flushed) { const ulong wd_limit = ...; flushed += CONFIG_SYS_CACHELINE_SIZE; if (flushed >= wd_limit) { WATCHDOG_RESET(); flushed = 0; } return flushed; }
and "flushed = maybe_reset(flushed);"? I don't think that improves readability.
Yes, something like this. I agree that it's not really beautiful but for my personal taste it's a bit better. But I don't have hard feeling here. Let's see what other comment on this.
Thanks, Stefan