[RFC] introduce --param max-lto-partition for having an upper bound on partition size

[Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]

Hi,
The attached patch introduces param max-lto-partition which creates an upper
bound for partition size.

My primary motivation for this patch is to fix building chromium for arm
with -flto-partition=one.
Chromium fails to build with -flto-partition={none, one} with assembler error:
"branch out of range error"
because in both these cases LTO creates a single text section of 18 mb
which exceeds thumb's limit of 16 mb and arm backend emits a short
call if caller and callee are in same section.
This is binutils PR18625:
https://sourceware.org/bugzilla/show_bug.cgi?id=18625
With patch, chromium builds for -flto-partition=one (by creating more
than one but minimal number of partitions to honor 16 mb limit).
I haven't tested with -flto-partition=none but I suppose the build
will still fail for none, because it won't involve partitioning?  I am
not sure how to fix for none case.

As suggested by Jim in binutils PR18625, the proper fix would be to
implement branch relaxation in arm's port of gas, however I suppose
only LTO will realistically create such large sections,
and implementing branch relaxation appears to be quite complicated and
probably too much of
an effort for this single use case, so this patch serves as a
work-around to the issue.
I am looking into fine-tuning the param value for ARM backend to
roughly match limit
of 16 mb.

AFAIU, this would change semantics of --param n_lto_partitions (or
-flto-partition=one) from
"exactly n_lto_partitions" to "at-least n_lto_partitions". If that's
not desirable maybe we could add
another param/option ?
Cross-tested on arm*-*-*.
Would this patch be OK for stage-1 (after getting param value right
for ARM target) ?

Thanks,
Prathamesh

[-- Attachment #2: max-partition.diff --]
[-- Type: text/plain, Size: 2048 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c868490..f734d56 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3459,6 +3459,11 @@ arm_option_override (void)
 
   /* Init initial mode for testing.  */
   thumb_flipper = TARGET_THUMB;
+
+    maybe_set_param_value (MAX_PARTITION_SIZE,
+			  10000, /* FIXME: fine-tune this value to roughly match 16 mb limit.  */
+                           global_options.x_param_values,
+			   global_options_set.x_param_values);
 }
 
 static void
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index 9eb63c2..bc0c612 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -511,9 +511,20 @@ lto_balanced_map (int n_lto_partitions)
   varpool_order.qsort (varpool_node_cmp);
 
   /* Compute partition size and create the first partition.  */
+  if (PARAM_VALUE (MIN_PARTITION_SIZE) > PARAM_VALUE (MAX_PARTITION_SIZE))
+    fatal_error (input_location, "min partition size cannot be greater than max partition size");
+
   partition_size = total_size / n_lto_partitions;
   if (partition_size < PARAM_VALUE (MIN_PARTITION_SIZE))
     partition_size = PARAM_VALUE (MIN_PARTITION_SIZE);
+  else if (partition_size > PARAM_VALUE (MAX_PARTITION_SIZE))
+    {
+      n_lto_partitions = total_size / PARAM_VALUE (MAX_PARTITION_SIZE);
+      if (total_size % PARAM_VALUE (MAX_PARTITION_SIZE))
+	n_lto_partitions++;
+      partition_size = total_size / n_lto_partitions;
+    }
+
   npartitions = 1;
   partition = new_partition ("");
   if (symtab->dump_file)
diff --git a/gcc/params.def b/gcc/params.def
index 9362c15..b6055ff 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -1029,6 +1029,11 @@ DEFPARAM (MIN_PARTITION_SIZE,
 	  "Minimal size of a partition for LTO (in estimated instructions).",
 	  1000, 0, 0)
 
+DEFPARAM (MAX_PARTITION_SIZE,
+	  "lto-max-partition",
+	  "Maximal size of a partition for LTO (in estimated instructions).",
+	  INT_MAX, 0, INT_MAX)
+
 /* Diagnostic parameters.  */
 
 DEFPARAM (CXX_MAX_NAMESPACES_FOR_DIAGNOSTIC_HELP,

[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 302 bytes --]

2016-04-01  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

	* params.def (MAX_PARTITION_SIZE): New parameter.
	* lto/lto-partition.c (lto_balanced_map): Check if partition size is greater than
	max-lto-partition.
	* config/arm/arm.c (arm_option_override): Set parm max-lto-partition
	to 10000.