Bug 16172 - simple function generates an memmove() call instead of inlining
Summary: simple function generates an memmove() call instead of inlining
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.0.0
: P2 enhancement
Target Milestone: 4.1.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2004-06-24 07:24 UTC by Andrew Morton
Modified: 2022-12-01 03:35 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 2.95.3, 3.0.4, 3.2.3, 3.3.3, 3.4.0, 4.0.0
Last reconfirmed: 2005-12-24 20:37:49


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Morton 2004-06-24 07:24:11 UTC
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.
Comment 1 Andrew Pinski 2004-06-24 08:06:10 UTC
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.
Comment 2 Falk Hueffner 2004-06-24 08:52:03 UTC
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;
}
Comment 3 Andrew Pinski 2004-06-28 02:03:07 UTC
Actually the memmove is not a regression at all, the inlining of the function is though.
Comment 4 roger 2004-10-31 03:28:12 UTC
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
Comment 5 Andrew Pinski 2004-10-31 04:35:23 UTC
I can still reproduce it with Falk's testcase on powerpc-darwin on the mainline.
Comment 6 Andrew Pinski 2005-01-13 19:25:11 UTC
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;
}
Comment 7 Andrew Pinski 2005-04-06 17:31:51 UTC
Note I don't think it is an inappropiate call to memmove.
Comment 8 Andrew Pinski 2022-12-01 03:35:46 UTC
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