public inbox for dwz@sourceware.org
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: dwz@sourceware.org, jakub@redhat.com
Subject: [committed] Add --devel-partition-dups-opt
Date: Tue, 01 Jan 2019 00:00:00 -0000	[thread overview]
Message-ID: <20191118122632.GA28908@delia> (raw)

Hi,

There's an optimization condition here in partition_dups_1:
...
      /* If during second_phase there are some DIEs we want to force
         into a partial unit because they are referenced from something
         already forced into a partial unit, but also some DIEs with
         the same set of referrers, try to see if we can put also those
         into the partial unit.  They can be put there only if they
         don't refer to DIEs that won't be put into partial units.  */
      if (second_phase && force && force < j - k)
        {
...

However, due to a preceding loop:
...
      for (k = i; k < j; k++)
...
and a 'j > i' invariant at the start of that loop we'll always end up with
'j == k' at the condition, so the optimization will never be triggered.

The intention of the optimization condition is to test whether there are some
forced as well as some unforced DIEs in the partition [i,j).

The '&& force' part of the condition tests whether there are forced dies,
given that 'force' is the number of forced DIEs.

The '&& force < j - k' part of the condition is supposed to test whether there
are any unforced DIEs.  The correct way to test this is '&& force < j - i',
given that 'j - i' is the total number of DIEs in the partition.

However, with this fix in place, the optimization seems hard to trigger.  It
doesn't trigger in the regular testsuite, and while it triggers in the external
testsuite on vmlinux-4.20.13-0-vanilla.debug, the optimization has no
effect on the dwz output for that file.

In the absence of proof that the optimization is properly tested, classify the
optimization as experimental, and guard with --devel-partition-dups-opt.

Committed to trunk.

Thanks,
- Tom

Add --devel-partition-dups-opt

2019-11-18  Tom de Vries  <tdevries@suse.de>

	PR dwz/24399
	* dwz.c (partition_dups_opt): New var.
	(partition_dups_1): Fix optimization condition.  Guard
	optimization with partition_dups_opt.
	(dwz_options): Add --devel-partition-dups-opt entry.

---
 dwz.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/dwz.c b/dwz.c
index acf0541..500ac1a 100644
--- a/dwz.c
+++ b/dwz.c
@@ -161,6 +161,7 @@ static int unoptimized_multifile;
 static int save_temps = 0;
 static int verify_edges_p = 0;
 static int dump_edges_p = 0;
+static int partition_dups_opt;
 
 typedef struct
 {
@@ -5674,7 +5675,8 @@ partition_dups_1 (dw_die_ref *arr, size_t vec_size,
 	 the same set of referrers, try to see if we can put also those
 	 into the partial unit.  They can be put there only if they
 	 don't refer to DIEs that won't be put into partial units.  */
-      if (second_phase && force && force < j - k)
+      if (unlikely (partition_dups_opt)
+	  && second_phase && force && force < j - i)
 	{
 	  /* First optimistically assume all such DIEs can be put there,
 	     thus mark all such DIEs as going to be included, so that
@@ -12671,6 +12673,8 @@ static struct option dwz_options[] =
 			 no_argument,	    &unoptimized_multifile, 1 },
   { "devel-verify-edges",no_argument,	    &verify_edges_p, 1 },
   { "devel-dump-edges",  no_argument,	    &dump_edges_p, 1 },
+  { "devel-partition-dups-opt",
+			 no_argument,	    &partition_dups_opt, 1 },
 #endif
   { NULL,		 no_argument,	    0, 0 }
 };

                 reply	other threads:[~2019-11-18 12:26 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20191118122632.GA28908@delia \
    --to=tdevries@suse.de \
    --cc=dwz@sourceware.org \
    --cc=jakub@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).