[committed] Add --devel-partition-dups-opt

[committed] Add --devel-partition-dups-opt

[Tom de Vries]

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 }
 };