Re: [PATCH PR64434]

[Yuri Rumyantsev <ysrumyan@gmail.com>]

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

Hi All,

I did all changes proposed by Richard and delete check on def in the
same block as Jakub proposed.
I also moved check  on optimization to call site..

I also checked that bootstrap and regression testing did not show any
new failures.

Is it OK for trunk?

2015-01-14 14:02 GMT+03:00 Jakub Jelinek <jakub@redhat.com>:
> On Wed, Jan 14, 2015 at 11:58:50AM +0100, Richard Biener wrote:
>> >> +      /* Swap operands if the second one is more expensive.  */
>> >> +      def0 = get_gimple_for_ssa_name (op0);
>> >> +      if (!def0)
>> >> +     continue;
>> >> +      def1 = get_gimple_for_ssa_name (op1);
>> >> +      if (!def1)
>> >> +     continue;
>> >> +      swap = false;
>> >
>> > You don't check here if def0/def1 are from the same bb, is that guaranteed?
>>
>> I think so - we only TER inside BBs.
>
> But then why to check for it a few lines above:
>
> +         def_stmt = get_gimple_for_ssa_name (use);
> +         if (!def_stmt || gimple_bb (def_stmt) != bb)
>
> If get_gimple_for_ssa_name != NULL guarantees that gimple_bb of the result == bb, then
> even the || gimple_bb (def_stmt) != bb shouldn't be needed.
>
>         Jakub

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 3809 bytes --]

Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 219439)
+++ cfgexpand.c	(working copy)
@@ -4964,6 +4964,86 @@
   flag_strict_aliasing = save_strict_alias;
 }
 
+/* Performs swapping operands of commutative operations to expand
+   the expensive one first.  */
+
+static void
+reorder_operands (basic_block bb)
+{
+  unsigned int *lattice;  /* Hold cost of each statement.  */
+  unsigned int i = 0, n = 0;
+  gimple_stmt_iterator gsi;
+  gimple_seq stmts;
+  gimple stmt;
+  bool swap;
+  tree op0, op1;
+  ssa_op_iter iter;
+  use_operand_p use_p;
+  enum tree_code code;
+  gimple def0, def1;
+
+  /* Compute cost of each statement using estimate_num_insns.  */
+  stmts = bb_seq (bb);
+  for (gsi = gsi_start (stmts); !gsi_end_p (gsi); gsi_next (&gsi))
+    {
+      stmt = gsi_stmt (gsi);
+      gimple_set_uid (stmt, n++);
+    }
+  lattice = XALLOCAVEC (unsigned, n);
+  for (gsi = gsi_start (stmts); !gsi_end_p (gsi); gsi_next (&gsi))
+    {
+      unsigned cost;
+      stmt = gsi_stmt (gsi);
+      cost = estimate_num_insns (stmt, &eni_size_weights);
+      lattice[i] = cost;
+      FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_USE | SSA_OP_VUSE)
+	{
+	  tree use = USE_FROM_PTR (use_p);
+	  gimple def_stmt;
+	  if (TREE_CODE (use) != SSA_NAME)
+	    continue;
+	  def_stmt = get_gimple_for_ssa_name (use);
+	  if (!def_stmt)
+	    continue;
+	  lattice[i] += lattice[gimple_uid (def_stmt)];
+	}
+      i++;
+      if (!is_gimple_assign (stmt)
+	  || gimple_has_volatile_ops (stmt))
+	continue;
+      code = gimple_assign_rhs_code (stmt);
+      if (!commutative_tree_code (code))
+	continue;
+      gcc_assert (gimple_num_ops (stmt) == 3);
+      op0 = gimple_op (stmt, 1);
+      op1 = gimple_op (stmt, 2);
+      if (op0 == NULL_TREE || op1 == NULL_TREE
+	  || TREE_CODE (op0) != SSA_NAME
+	  || TREE_CODE (op1) != SSA_NAME)
+	continue;
+      /* Swap operands if the second one is more expensive.  */
+      def0 = get_gimple_for_ssa_name (op0);
+      if (!def0)
+	continue;
+      def1 = get_gimple_for_ssa_name (op1);
+      if (!def1)
+	continue;
+      swap = false;
+      if (lattice[gimple_uid (def1)] > lattice[gimple_uid (def0)])
+	swap = true;
+      if (swap)
+	{
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    {
+	      fprintf (dump_file, "Swap operands in stmt:\n");
+	      print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
+	    }
+	  swap_ssa_operands (stmt, gimple_assign_rhs1_ptr (stmt),
+			     gimple_assign_rhs2_ptr (stmt));
+	}
+    }
+}
+
 /* Expand basic block BB from GIMPLE trees to RTL.  */
 
 static basic_block
@@ -4985,6 +5065,8 @@
      cannot use the gsi_*_bb() routines because they expect the basic
      block to be in GIMPLE, instead of RTL.  Therefore, we need to
      access the BB sequence directly.  */
+  if (optimize)
+    reorder_operands (bb);
   stmts = bb_seq (bb);
   bb->il.gimple.seq = NULL;
   bb->il.gimple.phi_nodes = NULL;
Index: testsuite/gcc.dg/torture/pr64434.c
===================================================================
--- testsuite/gcc.dg/torture/pr64434.c	(revision 0)
+++ testsuite/gcc.dg/torture/pr64434.c	(working copy)
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-rtl-expand-details" } */
+
+#define N 256
+int a1[N], a2[N], a3[N], a4[N];
+
+void foo ()
+{
+  int i;
+  for (i=0; i<N; i++) {
+    int c;
+    c = a3[i] + (a1[i] * a2[i]);
+    a4[i] = c + 1;
+    a1[i] = a2[i] - 1;
+  }
+}
+
+/* { dg-final { scan-rtl-dump-times "Swap operands" 1 "expand" } } */
+/* { dg-final { cleanup-rtl-dump "expand" } } */

Property changes on: testsuite/gcc.dg/torture/pr64434.c
___________________________________________________________________
Added: svn:executable
## -0,0 +1 ##
+*
\ No newline at end of property