[RFC] [P2] [PR tree-optimization/33562] Lowering more complex assignments.
Richard Biener
richard.guenther@gmail.com
Fri Feb 12 09:04:00 GMT 2016
On Fri, Feb 12, 2016 at 12:53 AM, Jeff Law <law@redhat.com> wrote:
>
> 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?
I think what you are missing is that you'll "optimize"
_Complex double x, y;
void foo (void)
{
x = y;
}
then but dependent on the targets capability DCmode moves might be
cheaper than four DFmode moves and nothing will combine them together
again.
In complex lowering we're careful to only "optimize" when we know how to
extract components in an efficient way (well, mostly).
That the DSE opportunities in complex-[45].c are exposed if you lower
the aggregate inits is obvious but the lowering is only a workaround for
our lack of handling for this case. I think the specific cases can be
easily made work by enhancing tree DSE in dse_possible_dead_store_p
to pick up partial kills by adjusting *ref (its offset/size) - keeping the
original ref for picking up uses.
But really we simply need a better DSE algorithm.
I think the two testcases are quite artificial and any "workaround" we
implement will cause more regressions elsewhere (code-size, RA, etc.)
if there are no followup optimization opportunities.
Richard.
> 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:
>
More information about the Gcc-patches
mailing list