On 04/01/2013 02:08 AM, Martin Decky wrote: >>> builds that use -O2 -ftree-loop-distribute-patterns show the original >>> problem >> >> Thanks, this helped. I committed the fix to mainline,1789. Looks like >> there can be a problem with memmove() too. > > I don't get it. In what sense is using such a highly-specific > optimization attribute and sticking to the sub-optimal C implementations > of the mem*() functions the _best_ solution? > > I have the assembler versions of memset() for all platforms almost > ready, we already have memcpy() for all platforms. So what's the point > of reverting back to revision 1783?
There are a few points to consider: - the fix is generic and only requires each prototype of these risky functions (memset(),memcpy(),memove(),others?) to be decorated by the attribute; the attribute is exactly the one which causes/solves the trouble (is even documented as such, so we are not suppressing some side-effect of it) and can be abstracted away in some compiler-dependent macro, such as COMPILER_RECOGNIZED_PATTERN - on the other hand, we would have to provide asm implementations for triplets [boot, kernel, uspace] of 7 x (3 + ?) functions, out of which we only have memcpy() for all platforms and now you report you are close to having memset() for all; how about memmove()? Are we going to rewrite all functions that are 'recognized' by gcc into asm? I think this is highly impractical - you say sub-optimal C implementations; it seems to me that the compiler is very well aware of what is the function doing and will optimize the function heavily anyway (-O3 still applies here - just see the difference between a -O0 and -O3 version), only in a non-recursive way; it looks easier to me to write an optimized eg. memmove() in C than an optimized memmove() in 7 assemblers by hand - we have had bugs in our C memmove() implementation: easy to fix in C, much more difficult to fix in all asm implementations; or conversely, we can have a latent bug in only one of the hand-written asm implementations (hasn't this been already the case of a very hard-to-find bug in our mips32 memcpy_from/to_uspace?); I see this as a maintenance burden - anyway, this universal fix can be considered only a stop-gap solution which can be easily reverted once we have all the asm implementations ready and believe they provide the best solution; having this solution in place allowed me to see another problem exhibited by the new toolchain: mips32, ia64 and sparc64 have an alignment issue in fat_lfn_size() Jakub _______________________________________________ HelenOS-devel mailing list [email protected] http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
