On Mon, Dec 4, 2017 at 3:43 PM, Richard Biener <rguent...@suse.de> wrote:
>
> When skimming through the code I noticed the following (chatted on IRC
> about parts of the changes).
>
> Bootstrap / regtest running on x86_64-unknown-linux-gnu.
>
> Will commit tomorrow unless you beat me to that.
>
> Richard.
>
> 2017-12-04 Richard Biener <rguent...@suse.de>
>
> * gimple-loop-interchange.cc (loop_cand::classify_simple_reduction):
> Simplify.
> (loop_cand::analyze_iloop_reduction_var): Reject dead reductions.
> (loop_cand::analyze_oloop_reduction_var): Likewise. Simplify.
> (tree_loop_interchange::interchange_loops): Properly analyze
> scalar evolution before instantiating a SCEV.
>
> Index: gcc/gimple-loop-interchange.cc
> ===================================================================
> --- gcc/gimple-loop-interchange.cc (revision 255383)
> +++ gcc/gimple-loop-interchange.cc (working copy)
> @@ -444,50 +444,21 @@ loop_cand::classify_simple_reduction (re
> if (!bb || bb->loop_father != m_outer)
> return;
>
> - if (!is_gimple_assign (producer))
> + if (!gimple_assign_load_p (producer))
> return;
>
> - code = gimple_assign_rhs_code (producer);
> - if (get_gimple_rhs_class (code) != GIMPLE_SINGLE_RHS)
> - return;
> -
> - lhs = gimple_assign_lhs (producer);
> - if (lhs != re->init)
> - return;
> -
> - rhs = gimple_assign_rhs1 (producer);
> - if (!REFERENCE_CLASS_P (rhs))
> - return;
> -
> - re->init_ref = rhs;
> + re->init_ref = gimple_assign_rhs1 (producer);
> }
> else if (!CONSTANT_CLASS_P (re->init))
> return;
>
> - /* Check how reduction variable is used. Note usually reduction variable
> - is used outside of its defining loop, we don't require that in terms of
> - loop interchange. */
> - if (!re->lcssa_phi)
> - consumer = single_use_in_loop (re->next, m_loop);
> - else
> - consumer = single_use_in_loop (PHI_RESULT (re->lcssa_phi), m_outer);
> -
> - if (!consumer || !is_gimple_assign (consumer))
> - return;
> -
> - code = gimple_assign_rhs_code (consumer);
> - if (get_gimple_rhs_class (code) != GIMPLE_SINGLE_RHS)
> - return;
> -
> - lhs = gimple_assign_lhs (consumer);
> - if (!REFERENCE_CLASS_P (lhs))
> - return;
> -
> - rhs = gimple_assign_rhs1 (consumer);
> - if (rhs != PHI_RESULT (re->lcssa_phi))
> + /* Check how reduction variable is used. */
> + consumer = single_use_in_loop (PHI_RESULT (re->lcssa_phi), m_outer);
> + if (!consumer
> + || !gimple_store_p (consumer))
> return;
>
> - re->fini_ref = lhs;
> + re->fini_ref = gimple_get_lhs (consumer);
> re->consumer = consumer;
>
> /* Simple reduction with constant initializer. */
> @@ -608,6 +579,9 @@ loop_cand::analyze_iloop_reduction_var (
> else
> return false;
> }
> + if (!lcssa_phi)
> + return false;
> +
> re = XCNEW (struct reduction);
> re->var = var;
> re->init = init;
> @@ -681,15 +655,9 @@ loop_cand::analyze_oloop_reduction_var (
>
> /* Outer loop's reduction should only be used to initialize inner loop's
> simple reduction. */
> - FOR_EACH_IMM_USE_FAST (use_p, iterator, var)
> - {
> - stmt = USE_STMT (use_p);
> - if (is_gimple_debug (stmt))
> - continue;
> -
> - if (stmt != inner_re->phi)
> - return false;
> - }
> + if (! single_imm_use (var, &use_p, &stmt)
> + || stmt != inner_re->phi)
> + return false;
>
> /* Check this reduction is correctly used outside of loop via lcssa phi.
> */
> FOR_EACH_IMM_USE_FAST (use_p, iterator, next)
> @@ -711,6 +679,8 @@ loop_cand::analyze_oloop_reduction_var (
> else
> return false;
> }
> + if (!lcssa_phi)
> + return false;
>
> re = XCNEW (struct reduction);
> re->var = var;
> @@ -1146,12 +1116,18 @@ tree_loop_interchange::interchange_loops
> edge instantiate_below = loop_preheader_edge (loop_nest);
> gsi = gsi_last_bb (loop_preheader_edge (loop_nest)->src);
> i_niters = number_of_latch_executions (iloop.m_loop);
> - i_niters = instantiate_scev (instantiate_below, loop_nest, i_niters);
> + i_niters = analyze_scalar_evolution (loop_outer (iloop.m_loop), i_niters);
> + i_niters = instantiate_scev (instantiate_below, loop_outer (iloop.m_loop),
> + i_niters);
> i_niters = force_gimple_operand_gsi (&gsi, unshare_expr (i_niters), true,
> NULL_TREE, false,
> GSI_CONTINUE_LINKING);
> o_niters = number_of_latch_executions (oloop.m_loop);
> if (oloop.m_loop != loop_nest)
> - o_niters = instantiate_scev (instantiate_below, loop_nest, o_niters);
> + {
> + o_niters = analyze_scalar_evolution (loop_outer (oloop.m_loop),
> o_niters);
> + o_niters = instantiate_scev (instantiate_below, loop_outer
> (oloop.m_loop),
> + o_niters);
> + }
Hmm, sorry to disturb. IIRC, it's important to instantiate niters
against the outermost loop in nest. Otherwise niters could refer to
intermediate variables defined by loop in nest, which may lead to
undefined ssa use issue in the chain of interchange.
Thanks,
bin
> o_niters = force_gimple_operand_gsi (&gsi, unshare_expr (o_niters), true,
> NULL_TREE, false,
> GSI_CONTINUE_LINKING);
>
