diff mbox

PR69619: Fix exponential issue in ccmp.c

Message ID AM3PR08MB0088F9EB9D134635F586801F83D00@AM3PR08MB0088.eurprd08.prod.outlook.com
State New
Headers show

Commit Message

Wilco Dijkstra Feb. 3, 2016, 4:35 p.m. UTC 
This patch fixes an exponential issue in ccmp.c.  When deciding which ccmp
expansion to use, the tree nodes gs0 and gs1 are fully expanded twice.  If
they contain more CCMP opportunities, their subtrees are also expanded twice.
When the trees are complex the expansion takes exponential time and memory.
As a workaround in GCC6 compute the cost of the first expansion early, and
only try the alternative expansion if the cost is low enough.  This rarely
affects real code, eg. SPECINT2006 has identical codesize.

For GCC7 we should improve the way this works and simplify the backend
interface.  I don't see why the backend should expand tree expressions,
especially when they are not part of the CCMP sequence.

OK for commit?

ChangeLog:
2016-02-03  Wilco Dijkstra  <wdijkstr@arm.com>

    gcc/
	PR target/69619
	* ccmp.c (expand_ccmp_expr_1): Avoid evaluating gs0/gs1
	twice when complex.

    gcc/testsuite/

	* gcc.dg/pr69619.c: Add new test.

Comments

Bernd Schmidt Feb. 4, 2016, 12:23 a.m. UTC | #1 
On 02/03/2016 05:35 PM, Wilco Dijkstra wrote:
> -	  tmp2 = targetm.gen_ccmp_first (&prep_seq_2, &gen_seq_2, rcode1,
> -					 gimple_assign_rhs1 (gs1),
> -					 gimple_assign_rhs2 (gs1));
> -

It looks like after this patch tmp2 could be used uninitialized? Should 
be fixed.

> +
> +	  /* FIXME: Temporary workaround for PR69619.
> +	     Avoid exponential compile time due to expanding gs0 and gs1 twice.
> +	     If gs0 and gs1 are complex, the cost will be high, so avoid
> +	     reevaluation if above an arbitrary threshold.  */
> +	  if ((tmp == NULL) || (cost1 < 100))

Two sets of unnecessary parentheses. Also, I think the cost should be 
based on COSTS_N_INSNS for proper units.

Otherwise I think this is a reasonable workaround for this stage. Ok 
with these changes.


Bernd
diff mbox

Patch

diff --git a/gcc/ccmp.c b/gcc/ccmp.c
index 9f1ce295554d17c0c3e39676632a07cabe7d5493..dce610488f2d13d6983f3752fb884c8af7ed3bc8 100644
--- a/gcc/ccmp.c
+++ b/gcc/ccmp.c
@@ -183,19 +183,25 @@  expand_ccmp_expr_1 (gimple *g, rtx *prep_seq, rtx *gen_seq)
 					gimple_assign_rhs1 (gs0),
 					gimple_assign_rhs2 (gs0));
 
-	  tmp2 = targetm.gen_ccmp_first (&prep_seq_2, &gen_seq_2, rcode1,
-					 gimple_assign_rhs1 (gs1),
-					 gimple_assign_rhs2 (gs1));
-
-	  if (!tmp && !tmp2)
-	    return NULL_RTX;
-
 	  if (tmp != NULL)
 	    {
 	      ret = expand_ccmp_next (gs1, code, tmp, &prep_seq_1, &gen_seq_1);
 	      cost1 = seq_cost (safe_as_a <rtx_insn *> (prep_seq_1), speed_p);
 	      cost1 += seq_cost (safe_as_a <rtx_insn *> (gen_seq_1), speed_p);
 	    }
+
+	  /* FIXME: Temporary workaround for PR69619.
+	     Avoid exponential compile time due to expanding gs0 and gs1 twice.
+	     If gs0 and gs1 are complex, the cost will be high, so avoid
+	     reevaluation if above an arbitrary threshold.  */
+	  if ((tmp == NULL) || (cost1 < 100))
+	    tmp2 = targetm.gen_ccmp_first (&prep_seq_2, &gen_seq_2, rcode1,
+					   gimple_assign_rhs1 (gs1),
+					   gimple_assign_rhs2 (gs1));
+
+	  if (!tmp && !tmp2)
+	    return NULL_RTX;
+
 	  if (tmp2 != NULL)
 	    {
 	      ret2 = expand_ccmp_next (gs0, code, tmp2, &prep_seq_2,
diff --git a/gcc/testsuite/gcc.dg/pr69619.c b/gcc/testsuite/gcc.dg/pr69619.c
new file mode 100644
index 0000000000000000000000000000000000000000..a200bdf310fc2c02b008e0c13fb9c917784423f8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69619.c
@@ -0,0 +1,20 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+int a, b, c, d;
+int e[100];
+void
+fn1 ()
+{
+  int *f = &d;
+  c = 6;
+  for (; c; c--)
+    {
+      b = 0;
+      for (; b <= 5; b++)
+	{
+	  short g = e[(b + 2) * 9 + c];
+	  *f = *f == a && e[(b + 2) * 9 + c];
+	}
+    }
+}