So I've never thought much about our Complex support and I don't have a lot of confidence in the coverage for our testsuite for these changes. So I'd really appreciate someone with more experience thinking about this issue for a bit.

I was looking at 33562 (P2), figuring it was DSE, which I wrote ~15 years ago, I could probably get up-to-speed and so something about it without major surgery (and for Complex I'm pretty sure I can).

But while looking at the gimple code we handed off to DSE, it occurred to me that this would be easy to optimize if it were lowered. Then, of course i remembered that we did lower complex stuff.

So this turned into a short debugging session in tree-complex.c.

The key statement in the test looks like



complex int t = 0

Where x is a complex object *and* has its address taken. So the IL looks something like:

t = __complex__ (0, 0);



init_dont_simulate_again ignores this statement because the LHS is not an SSA_NAME (remember, t has had its address taken elsewhere in the sources).

So complex lowering ends up totally ignoring the function in question.

ISTM that we can and should still lower this code. We don't want to set sim_again_p because the LHS is not in SSA form, so we don't really want/need to set up and track a lattice for this object.

So the first step was to figure out the conditions under which we ought to detect an assignment to/from an aggregate that is not in SSA_FORM.

I think we can do that with something like this in the GIMPLE_ASSIGN case:
             /* Simple assignments involving complex types where
                 the RHS or LHS is addressable should be lowered, but
                 do not inherently trigger resimulation.  */
              if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt)))
                  == COMPLEX_TYPE)
                saw_a_complex_op = true;


Essentially anytime we see a simple assignment (which would include initialization) we go ahead and let the complex lowering pass run, but we don't set sim_again_p.


Then we need to actually lower the construct. expand_complex_move has 99% of what we need. If we take the code from this clause:

   else if (rhs && TREE_CODE (rhs) == SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
[ ... ]

Refactor it into its own function and call it when this condition is true:

+  /* Assignment to/from a complex object that is addressable.  */
+  else if (TREE_CODE (lhs) == VAR_DECL
+          && TREE_CODE (TREE_TYPE (lhs)) == COMPLEX_TYPE
+          && (TREE_CODE (rhs) == VAR_DECL
+              || TREE_CODE (rhs) == COMPLEX_CST
+              || TREE_CODE (rhs) == COMPLEX_EXPR)
+          && TREE_CODE (TREE_TYPE (rhs)) == COMPLEX_TYPE)

Then complex-4.c and complex-5.c both work. A variant of complex-4.c that I hacked up were we pass in a complex int, and assign that to a local addressable complex int gets lowered (and better optimized) as well.

So what am I missing here? Is there any kind of aliasing issues I need to be aware of? Any other dragons lurking?
jeff




diff --git a/gcc/tree-complex.c b/gcc/tree-complex.c
index d781a8a..64346a5 100644
--- a/gcc/tree-complex.c
+++ b/gcc/tree-complex.c
@@ -240,6 +240,14 @@ init_dont_simulate_again (void)
                op0 = gimple_assign_rhs1 (stmt);
              if (gimple_num_ops (stmt) > 2)
                op1 = gimple_assign_rhs2 (stmt);
+
+             /* Simple assignments involving complex types where
+                the RHS or LHS is addressable should be lowered, but
+                do not inherently trigger resimulation.  */
+             if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt)))
+                 == COMPLEX_TYPE)
+               saw_a_complex_op = true;
+
              break;
 
            case GIMPLE_COND:
@@ -777,6 +785,67 @@ update_phi_components (basic_block bb)
     }
 }
 
+/* Extract the real and imag parts from RHS of the statement at GSI
+   into R and I respectively.
+
+   This wouldn't be necessary except that extracting these
+   components from a COMPLEX_EXPR is different from everything
+   else.  */
+
+static void
+extract_real_and_imag_component (gimple_stmt_iterator *gsi, tree *r, tree *i)
+{
+  gimple *stmt = gsi_stmt (*gsi);
+  if (gimple_assign_rhs_code (stmt) == COMPLEX_EXPR)
+    {
+      *r = gimple_assign_rhs1 (stmt);
+      *i = gimple_assign_rhs2 (stmt);
+    }
+  else
+    {
+      tree tmp = gimple_assign_rhs1 (stmt);
+      *r = extract_component (gsi, tmp, 0, true);
+      *i = extract_component (gsi, tmp, 1, true);
+    }
+}
+
+/* Helper for expand_move_complex.  */
+
+static void
+expand_complex_move_1 (gimple_stmt_iterator *gsi, gimple *stmt,
+                      tree lhs, tree inner_type)
+{
+  tree x, r, i;
+  gimple *t;
+  location_t loc = gimple_location (stmt);
+
+  extract_real_and_imag_component (gsi, &r, &i);
+  x = build1 (REALPART_EXPR, inner_type, unshare_expr (lhs));
+  t = gimple_build_assign (x, r);
+  gimple_set_location (t, loc);
+  gsi_insert_before (gsi, t, GSI_SAME_STMT);
+
+  if (stmt == gsi_stmt (*gsi))
+    {
+      x = build1 (IMAGPART_EXPR, inner_type, unshare_expr (lhs));
+      gimple_assign_set_lhs (stmt, x);
+      gimple_assign_set_rhs1 (stmt, i);
+    }
+  else
+    {
+      x = build1 (IMAGPART_EXPR, inner_type, unshare_expr (lhs));
+      t = gimple_build_assign (x, i);
+      gimple_set_location (t, loc);
+      gsi_insert_before (gsi, t, GSI_SAME_STMT);
+
+      stmt = gsi_stmt (*gsi);
+      gcc_assert (gimple_code (stmt) == GIMPLE_RETURN);
+      gimple_return_set_retval (as_a <greturn *> (stmt), lhs);
+    }
+
+  update_stmt (stmt);
+}
+
 /* Expand a complex move to scalars.  */
 
 static void
@@ -829,54 +898,20 @@ expand_complex_move (gimple_stmt_iterator *gsi, tree type)
        }
       else
        {
-         if (gimple_assign_rhs_code (stmt) != COMPLEX_EXPR)
-           {
-             r = extract_component (gsi, rhs, 0, true);
-             i = extract_component (gsi, rhs, 1, true);
-           }
-         else
-           {
-             r = gimple_assign_rhs1 (stmt);
-             i = gimple_assign_rhs2 (stmt);
-           }
+         extract_real_and_imag_component (gsi, &r, &i);
          update_complex_assignment (gsi, r, i);
        }
     }
   else if (rhs && TREE_CODE (rhs) == SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
-    {
-      tree x;
-      gimple *t;
-      location_t loc;
-
-      loc = gimple_location (stmt);
-      r = extract_component (gsi, rhs, 0, false);
-      i = extract_component (gsi, rhs, 1, false);
-
-      x = build1 (REALPART_EXPR, inner_type, unshare_expr (lhs));
-      t = gimple_build_assign (x, r);
-      gimple_set_location (t, loc);
-      gsi_insert_before (gsi, t, GSI_SAME_STMT);
-
-      if (stmt == gsi_stmt (*gsi))
-       {
-         x = build1 (IMAGPART_EXPR, inner_type, unshare_expr (lhs));
-         gimple_assign_set_lhs (stmt, x);
-         gimple_assign_set_rhs1 (stmt, i);
-       }
-      else
-       {
-         x = build1 (IMAGPART_EXPR, inner_type, unshare_expr (lhs));
-         t = gimple_build_assign (x, i);
-         gimple_set_location (t, loc);
-         gsi_insert_before (gsi, t, GSI_SAME_STMT);
-
-         stmt = gsi_stmt (*gsi);
-         gcc_assert (gimple_code (stmt) == GIMPLE_RETURN);
-         gimple_return_set_retval (as_a <greturn *> (stmt), lhs);
-       }
-
-      update_stmt (stmt);
-    }
+    expand_complex_move_1 (gsi, stmt, lhs, inner_type);
+  /* Initialization of a complex object that is addressable.  */
+  else if (TREE_CODE (lhs) == VAR_DECL
+          && TREE_CODE (TREE_TYPE (lhs)) == COMPLEX_TYPE
+          && (TREE_CODE (rhs) == VAR_DECL
+              || TREE_CODE (rhs) == COMPLEX_CST
+              || TREE_CODE (rhs) == COMPLEX_EXPR)
+          && TREE_CODE (TREE_TYPE (rhs)) == COMPLEX_TYPE)
+    expand_complex_move_1 (gsi, stmt, lhs, inner_type);
 }
 
 /* Expand complex addition to scalars:

Reply via email to