All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.