From: Arnaldo Carvalho de Melo <acme-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: "Ilpo Järvinen" <ilpo.jarvinen-pxSi+dnQzZMxHbG02/KK1g@public.gmane.org>
Cc: dwarves-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: codiff misses changes if inline -> not inlined?
Date: Thu, 3 Jan 2008 22:27:46 -0200 [thread overview]
Message-ID: <20080104002746.GR29523@ghostprotocols.net> (raw)
In-Reply-To: <Pine.LNX.4.64.0801040111530.16761-Y/UOj9v5BLQhZigby9b+C6cUovnZ0M2TMR2xtNvyitY@public.gmane.org>
Em Fri, Jan 04, 2008 at 01:24:02AM +0200, Ilpo Järvinen escreveu:
> On Thu, 3 Jan 2008, Arnaldo Carvalho de Melo wrote:
>
> > Em Thu, Jan 03, 2008 at 03:40:16PM +0200, Ilpo Järvinen escreveu:
> > > Hi,
> > >
> > > I've had a problem with codiff when playing around with inlines. It seems
> > > to miss completely the resizement of inlined function from zero to
> > > something. Here's one example (relevant patch attached):
> > >
> > > $ codiff tcp_input.o.old tcp_input.o
> > > net/ipv4/tcp_input.c:
> > > tcp_dsack_extend | -73
> > > tcp_data_queue | -17
> > > 2 functions changed, 90 bytes removed
> > >
> > > $ pfunct -s tcp_input.o.old | grep "tcp_sack_extend"
> > > $ pfunct -s tcp_input.o | grep "tcp_sack_extend"
> > > tcp_sack_extend: 66
> > >
> > > Isn't that tcp_sack_extend | +66 missing from codiff's output???
> > >
> > >
> > > There's nothing special in the tcp_sack_extend(), any inline (at least in
> > > .c files) will do.
> >
> > From codiff's point of view its a "new" function... I'm checking this right now, just a sec :)
>
> More very interesting cases found... :-/ The tool removed inline from
> ip_ufo_append_data and got this result:
>
> $ codiff -V net/ipv4/ip_output.o.old net/ipv4/ip_output.o
> net/ipv4/ip_output.c:
> dst_output | +11 (uninlined)
> ip_send_check | +69 (uninlined)
> ip_finish_output2 | +594 (uninlined)
> 3 functions changed, 674 bytes added, diff: +674
>
> ...It may well be right due to some strange inlining heurestics
> interaction but I'm a bit suspicious because of the large numbers and
> pfunct -i tells a different story for at least ip_finish_output2 and
> ip_send_check (in both cases the sizes are constant 0 in -i output!?!).
Before manually removing inline from the function:
[acme@doppio net-2.6.25]$ pfunct -f ip_ufo_append_data /tmp/ip_output.o.old
inline int ip_ufo_append_data(struct sock * sk, int (*getfrag)(void *, char *, int, int, int, struct sk_buff *), void * from, int length, int hh_len, int fragheaderlen, int transhdrlen, int mtu, unsigned int flags);
[acme@doppio net-2.6.25]$
And the compiler is not, at least in my kernel configuration, trying to
do (un)inlining differently from what is in the source code:
[acme@doppio net-2.6.25]$ pfunct --cc_uninlined /tmp/ip_output.o.old
[acme@doppio net-2.6.25]$ pfunct --cc_inlined /tmp/ip_output.o.old
[acme@doppio net-2.6.25]$
[acme@doppio net-2.6.25]$ grep INLIN ../build/net-2.6.25/doppio/.config
# CONFIG_FORCED_INLINING is not set
[acme@doppio net-2.6.25]$ pfunct -T -f ip_append_data /tmp/ip_output.o.old
Shows that ip_ufo_append_data is being inlined in ip_append_data, taking
336 bytes in my config (x86_64).
Then I remove the inline and...
[acme@doppio net-2.6.25]$ codiff /tmp/ip_output.o.old
/tmp/ip_output.o.new
/home/acme/git/net-2.6.25/net/ipv4/ip_output.c:
dst_output | +14
ip_send_check | +65
ip_finish_output2 | +499
3 functions changed, 578 bytes added, diff: +578
[acme@doppio net-2.6.25]$
Which should have shown ip_ufo_append_data as being uninlined... ah!
[acme@doppio pahole]$ pfunct --cc_inlined /tmp/ip_output.o.new
ip_ufo_append_data
The compiler decided that even if we remove the 'inline' keyword the
function is only called from ip_append_data, so it inlines it anyway.
But why we end up using 578 bytes more? Lemme see...
It is a bug:
[acme@doppio pahole]$ size /tmp/ip_output.o.old /tmp/ip_output.o.new
text data bss dec hex filename
12262 4 0 12266 2fea /tmp/ip_output.o.old
12262 4 0 12266 2fea /tmp/ip_output.o.new
[acme@doppio pahole]$
Investigating...
- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2008-01-04 0:27 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <Pine.LNX.4.64.0801031528400.14949@kivilampi-30.cs.helsinki.fi>
[not found] ` <Pine.LNX.4.64.0801031528400.14949-Y/UOj9v5BLQhZigby9b+C6cUovnZ0M2TMR2xtNvyitY@public.gmane.org>
2008-01-03 15:10 ` codiff misses changes if inline -> not inlined? Arnaldo Carvalho de Melo
[not found] ` <20080103151044.GD29523-f8uhVLnGfZaxAyOMLChx1axOck334EZe@public.gmane.org>
2008-01-03 16:45 ` Ilpo Järvinen
[not found] ` <Pine.LNX.4.64.0801031728330.14949-Y/UOj9v5BLQhZigby9b+C6cUovnZ0M2TMR2xtNvyitY@public.gmane.org>
2008-01-03 16:56 ` Arnaldo Carvalho de Melo
[not found] ` <20080103165605.GF29523-f8uhVLnGfZaxAyOMLChx1axOck334EZe@public.gmane.org>
2008-01-03 22:41 ` Ilpo Järvinen
[not found] ` <Pine.LNX.4.64.0801040038510.20866-Y/UOj9v5BLQhZigby9b+C6cUovnZ0M2TMR2xtNvyitY@public.gmane.org>
2008-01-03 23:29 ` Arnaldo Carvalho de Melo
2008-01-03 23:30 ` Arnaldo Carvalho de Melo
[not found] ` <20080103143139.GC29523@ghostprotocols.net>
[not found] ` <20080103143139.GC29523-f8uhVLnGfZaxAyOMLChx1axOck334EZe@public.gmane.org>
2008-01-03 23:24 ` Ilpo Järvinen
[not found] ` <Pine.LNX.4.64.0801040111530.16761-Y/UOj9v5BLQhZigby9b+C6cUovnZ0M2TMR2xtNvyitY@public.gmane.org>
2008-01-03 23:35 ` Arnaldo Carvalho de Melo
2008-01-04 0:27 ` Arnaldo Carvalho de Melo [this message]
[not found] ` <20080104002746.GR29523-f8uhVLnGfZaxAyOMLChx1axOck334EZe@public.gmane.org>
2008-01-04 1:39 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080104002746.GR29523@ghostprotocols.net \
--to=acme-h+wxahxf7alqt0dzr+alfa@public.gmane.org \
--cc=dwarves-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=ilpo.jarvinen-pxSi+dnQzZMxHbG02/KK1g@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.