compiling http://www.zip.com.au/~akpm/linux/patches/stuff/fib_hash.i with /usr/local/gcc-cvs/bin/gcc -Wp,-MD,net/ipv4/.fib_hash.o.d -nostdinc -iwithprefix include -D__KERNEL__ -Iinclude -Wall -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -pipe -msoft-float -mpreferred-stack-boundary=2 -fno-unit-at-a-time -march=i686 -mregparm=3 -Iinclude/asm-i386/mach-default -gdwarf-2 -O1 -Os -g -Wdeclaration-after-statement -DKBUILD_BASENAME=fib_hash -DKBUILD_MODNAME=fib_hash -c -o net/ipv4/.tmp_fib_hash.o net/ipv4/fib_hash.c causes the fn_hash() function to generate a call to memmove(). It seems inappropriate, as that function deals with little scalars.
This does not make sense: return *(fn_hash_idx_t*)&h; why do it this way, to get NRV to work well for 3.5.0 this is not needed at all and is what caused the problem, also this is undefined by the C standard (but we define it with -fno-strict-aliasing but this just makes the code ugly to look at). Also why have it return a struct in the first place this seems dumb. Anyways a workaround (maybe even a real fix): fn_hash_idx_t t; t.datum = h; return t; But anyways the call to memmove is a regression.
Just for the record, here's a test case: struct fn_hash_idx_t { unsigned datum; }; struct fn_hash_idx_t fn_hash(unsigned h) { return *(struct fn_hash_idx_t*)&h; }
Actually the memmove is not a regression at all, the inlining of the function is though.
I'm unable to reproduce this using Falk's testcase in comment #2, and the original URL is now a broken link. Has the problem now been fixed, or could someone attach a failing testcase to the bugzilla PR. Please? TIA
I can still reproduce it with Falk's testcase on powerpc-darwin on the mainline.
Here is a testcase which I can reproduce it with on powerpc-darwin (it just has more elements in the struct): struct fn_hash_idx_t { unsigned datum; unsigned datum2; unsigned datum3; unsigned datumi4; unsigned datum5; unsigned datum6; unsigned datum7; unsigned datum8; unsigned datum9; }; struct fn_hash_idx_t fn_hash(unsigned h) { return *(struct fn_hash_idx_t*)&h; }
Note I don't think it is an inappropiate call to memmove.
The original testcase (in comment #2 as the fib_hash.i is gone) has been fixed for a long time now. At least GCC 4.1.2. So closing as fixed. Starting in GCC 10, the testcase produces even smaller code: fn_hash: .LFB0: .cfi_startproc movl %edx, (%eax) ret