public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][gimple-interchange] Random cleanups
@ 2017-12-04 15:43 Richard Biener
  2017-12-04 16:01 ` Bin.Cheng
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2017-12-04 15:43 UTC (permalink / raw)
  To: gcc-patches; +Cc: amker.cheng


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  <rguenther@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);
+    }
   o_niters = force_gimple_operand_gsi (&gsi, unshare_expr (o_niters), true,
 				       NULL_TREE, false, GSI_CONTINUE_LINKING);
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][gimple-interchange] Random cleanups
  2017-12-04 15:43 [PATCH][gimple-interchange] Random cleanups Richard Biener
@ 2017-12-04 16:01 ` Bin.Cheng
  2017-12-04 17:39   ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Bin.Cheng @ 2017-12-04 16:01 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches List

On Mon, Dec 4, 2017 at 3:43 PM, Richard Biener <rguenther@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  <rguenther@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);
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][gimple-interchange] Random cleanups
  2017-12-04 16:01 ` Bin.Cheng
@ 2017-12-04 17:39   ` Richard Biener
  2017-12-04 17:59     ` Bin.Cheng
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2017-12-04 17:39 UTC (permalink / raw)
  To: Bin.Cheng; +Cc: gcc-patches List

On December 4, 2017 5:01:45 PM GMT+01:00, "Bin.Cheng" <amker.cheng@gmail.com> wrote:
>On Mon, Dec 4, 2017 at 3:43 PM, Richard Biener <rguenther@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  <rguenther@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.


That's a common misconception of the instantiate_scev interface. We instantiate against below, the loop argument is to interpret the chrec and absolutely has to match what loop you called analyze_evolution on. Otherwise there's a chance you get garbage out (from garbage in). 

Richard. 
>Thanks,
>bin
>>    o_niters = force_gimple_operand_gsi (&gsi, unshare_expr
>(o_niters), true,
>>                                        NULL_TREE, false,
>GSI_CONTINUE_LINKING);
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][gimple-interchange] Random cleanups
  2017-12-04 17:39   ` Richard Biener
@ 2017-12-04 17:59     ` Bin.Cheng
  0 siblings, 0 replies; 4+ messages in thread
From: Bin.Cheng @ 2017-12-04 17:59 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches List

On Mon, Dec 4, 2017 at 5:39 PM, Richard Biener <rguenther@suse.de> wrote:
> On December 4, 2017 5:01:45 PM GMT+01:00, "Bin.Cheng" <amker.cheng@gmail.com> wrote:
>>On Mon, Dec 4, 2017 at 3:43 PM, Richard Biener <rguenther@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  <rguenther@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.
>
>
> That's a common misconception of the instantiate_scev interface. We instantiate against below, the loop argument is to interpret the chrec and absolutely has to match what loop you called analyze_evolution on. Otherwise there's a chance you get garbage out (from garbage in).
Ah, right.  Thanks for explaining.

Thanks,
bin
>
> Richard.
>>Thanks,
>>bin
>>>    o_niters = force_gimple_operand_gsi (&gsi, unshare_expr
>>(o_niters), true,
>>>                                        NULL_TREE, false,
>>GSI_CONTINUE_LINKING);
>>>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-12-04 17:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-04 15:43 [PATCH][gimple-interchange] Random cleanups Richard Biener
2017-12-04 16:01 ` Bin.Cheng
2017-12-04 17:39   ` Richard Biener
2017-12-04 17:59     ` Bin.Cheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).