Re: [RFA][PATCH] Use SCEV conditionally within vr-values and evrp range analysis - V2

[Richard Biener]

On Mon, Nov 27, 2017 at 5:43 PM, Jeff Law  wrote:
> On 11/23/2017 05:49 AM, Richard Biener wrote:
>> On Thu, Nov 23, 2017 at 1:16 AM, Jeff Law  wrote:
>>>
>>> Clients of the evrp range analysis may not have initialized the SCEV
>>> infrastructure, and in fact my not want to (DOM for example).
>>>
>>> Yet inside both vr-values.c and gimple-ssa-evrp-analyze.c we have calls
>>> into SCEV (that will fault/abort if SCEV is not properly initialized).
>>>
>>> This patch allows clients of vr-values.c and gimple-ssa-evrp-analyze.c
>>> to indicate if they want SCEV analysis.
>>>
>>> Bootstrapped and regression tested by itself as well as with the DOM
>>> patches to use EVRP analysis  (which test the "don't want SCEV path).
>>>
>>> OK for the trunk?
>>
>> There's also scev_initialized_p () which you could conveniently use.
> Yea, that worked fine and is (of course) much simpler.
>
> Bootstrapped and regression tested in isolation as well as on top of my
> ongoing work to remove jump threading from tree-vrp.c.
>
> OK for the trunk now?

Ok.

Richard.

> Jeff
>
>
> * gimple-ssa-evrp-analyze.c
> (evrp_range_analyzer::record_ranges_from_phis): Only use SCEV to
> refine ranges if scev_initialized_p returns true.
> * vr-values.c (vr_values::extract_range_from_phi_node): Likewise.
>
> diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
> index 68a2cdc..38fb0db 100644
> --- a/gcc/gimple-ssa-evrp-analyze.c
> +++ b/gcc/gimple-ssa-evrp-analyze.c
> @@ -176,7 +176,8 @@ evrp_range_analyzer::record_ranges_from_phis (basic_block 
> bb)
>  to use VARYING for them.  But we can still resort to
>  SCEV for loop header PHIs.  */
>   struct loop *l;
> - if (interesting
> + if (scev_initialized_p ()
> + && interesting
>   && (l = loop_containing_stmt (phi))
>   && l->header == gimple_bb (phi))
>   vr_values.adjust_range_with_scev (_result, l, phi, lhs);
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 2d11861..e617556 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -2935,7 +2935,8 @@ scev_check:
>   scev_check can be reached from two paths, one is a fall through from 
> above
>   "varying" label, the other is direct goto from code block which tries to
>   avoid infinite simulation.  */
> -  if ((l = loop_containing_stmt (phi))
> +  if (scev_initialized_p ()
> +  && (l = loop_containing_stmt (phi))
>&& l->header == gimple_bb (phi))
>  adjust_range_with_scev (vr_result, l, phi, lhs);
>
>

Re: [RFA][PATCH] Use SCEV conditionally within vr-values and evrp range analysis - V2

[Jeff Law]

On 11/23/2017 05:49 AM, Richard Biener wrote:
> On Thu, Nov 23, 2017 at 1:16 AM, Jeff Law  wrote:
>>
>> Clients of the evrp range analysis may not have initialized the SCEV
>> infrastructure, and in fact my not want to (DOM for example).
>>
>> Yet inside both vr-values.c and gimple-ssa-evrp-analyze.c we have calls
>> into SCEV (that will fault/abort if SCEV is not properly initialized).
>>
>> This patch allows clients of vr-values.c and gimple-ssa-evrp-analyze.c
>> to indicate if they want SCEV analysis.
>>
>> Bootstrapped and regression tested by itself as well as with the DOM
>> patches to use EVRP analysis  (which test the "don't want SCEV path).
>>
>> OK for the trunk?
> 
> There's also scev_initialized_p () which you could conveniently use.
Yea, that worked fine and is (of course) much simpler.

Bootstrapped and regression tested in isolation as well as on top of my
ongoing work to remove jump threading from tree-vrp.c.

OK for the trunk now?

Jeff

* gimple-ssa-evrp-analyze.c
(evrp_range_analyzer::record_ranges_from_phis): Only use SCEV to
refine ranges if scev_initialized_p returns true.
* vr-values.c (vr_values::extract_range_from_phi_node): Likewise.

diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
index 68a2cdc..38fb0db 100644
--- a/gcc/gimple-ssa-evrp-analyze.c
+++ b/gcc/gimple-ssa-evrp-analyze.c
@@ -176,7 +176,8 @@ evrp_range_analyzer::record_ranges_from_phis (basic_block 
bb)
 to use VARYING for them.  But we can still resort to
 SCEV for loop header PHIs.  */
  struct loop *l;
- if (interesting
+ if (scev_initialized_p ()
+ && interesting
  && (l = loop_containing_stmt (phi))
  && l->header == gimple_bb (phi))
  vr_values.adjust_range_with_scev (_result, l, phi, lhs);
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 2d11861..e617556 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -2935,7 +2935,8 @@ scev_check:
  scev_check can be reached from two paths, one is a fall through from above
  "varying" label, the other is direct goto from code block which tries to
  avoid infinite simulation.  */
-  if ((l = loop_containing_stmt (phi))
+  if (scev_initialized_p ()
+  && (l = loop_containing_stmt (phi))
   && l->header == gimple_bb (phi))
 adjust_range_with_scev (vr_result, l, phi, lhs);
 

Re: [RFA][PATCH] Use SCEV conditionally within vr-values and evrp range analysis

[Jeff Law]

On 11/23/2017 05:49 AM, Richard Biener wrote:
> On Thu, Nov 23, 2017 at 1:16 AM, Jeff Law  wrote:
>>
>> Clients of the evrp range analysis may not have initialized the SCEV
>> infrastructure, and in fact my not want to (DOM for example).
>>
>> Yet inside both vr-values.c and gimple-ssa-evrp-analyze.c we have calls
>> into SCEV (that will fault/abort if SCEV is not properly initialized).
>>
>> This patch allows clients of vr-values.c and gimple-ssa-evrp-analyze.c
>> to indicate if they want SCEV analysis.
>>
>> Bootstrapped and regression tested by itself as well as with the DOM
>> patches to use EVRP analysis  (which test the "don't want SCEV path).
>>
>> OK for the trunk?
> 
> There's also scev_initialized_p () which you could conveniently use.
Wasn't aware of it.  That's almost certainly a better solution.

Jeff

Re: [RFA][PATCH] Use SCEV conditionally within vr-values and evrp range analysis

[Richard Biener]

On Thu, Nov 23, 2017 at 1:16 AM, Jeff Law  wrote:
>
> Clients of the evrp range analysis may not have initialized the SCEV
> infrastructure, and in fact my not want to (DOM for example).
>
> Yet inside both vr-values.c and gimple-ssa-evrp-analyze.c we have calls
> into SCEV (that will fault/abort if SCEV is not properly initialized).
>
> This patch allows clients of vr-values.c and gimple-ssa-evrp-analyze.c
> to indicate if they want SCEV analysis.
>
> Bootstrapped and regression tested by itself as well as with the DOM
> patches to use EVRP analysis  (which test the "don't want SCEV path).
>
> OK for the trunk?

There's also scev_initialized_p () which you could conveniently use.

Richard.

>
> Jeff
>
> * gimple-ssa-evrp-analyze.h (class evrp_range_analyzer): New data
> member USE_SCEV.
> * gimple-ssa-evrp-analyze.c
> (evrp_range_analyzer::evrp_range_analyzer): Add new use_scev argument.
> Pass it to the vr_values ctor.
> (evrp_range_analyzer::record_ranges_from_phis): Conditionally use
> SCEV to refine ranges for PHIs.
> * gimple-ssa-evrp (evrp_dom_walker::evrp_dom_walker): Ask for
> SCEV analysis in the vr-values engine.
> * vr-values.h (class vr_values): New data meber USE_SCEV.
> * vr-values.c (vr_values::vr_values): Add new use_scev argument.
> (vr_values::extract_range_from_phi_node): Conditionally use
> SCEV to refine ranges for PHIs.
>
>
> diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
> index 68a2cdc..bc41be1 100644
> --- a/gcc/gimple-ssa-evrp-analyze.c
> +++ b/gcc/gimple-ssa-evrp-analyze.c
> @@ -42,7 +42,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "vr-values.h"
>  #include "gimple-ssa-evrp-analyze.h"
>
> -evrp_range_analyzer::evrp_range_analyzer () : stack (10)
> +evrp_range_analyzer::evrp_range_analyzer (bool use_scev_)
> +  : vr_values (use_scev_), stack (10), use_scev (use_scev_)
>  {
>edge e;
>edge_iterator ei;
> @@ -176,7 +177,8 @@ evrp_range_analyzer::record_ranges_from_phis (basic_block 
> bb)
>  to use VARYING for them.  But we can still resort to
>  SCEV for loop header PHIs.  */
>   struct loop *l;
> - if (interesting
> + if (use_scev
> + && interesting
>   && (l = loop_containing_stmt (phi))
>   && l->header == gimple_bb (phi))
>   vr_values.adjust_range_with_scev (_result, l, phi, lhs);
> @@ -341,3 +343,4 @@ evrp_range_analyzer::pop_value_range (tree var)
>stack.pop ();
>return vr;
>  }
> +
> diff --git a/gcc/gimple-ssa-evrp-analyze.h b/gcc/gimple-ssa-evrp-analyze.h
> index 3d64c50..18f5b8c 100644
> --- a/gcc/gimple-ssa-evrp-analyze.h
> +++ b/gcc/gimple-ssa-evrp-analyze.h
> @@ -23,7 +23,7 @@ along with GCC; see the file COPYING3.  If not see
>  class evrp_range_analyzer
>  {
>   public:
> -  evrp_range_analyzer (void);
> +  evrp_range_analyzer (bool);
>~evrp_range_analyzer (void) { stack.release (); }
>
>void enter (basic_block);
> @@ -65,6 +65,9 @@ class evrp_range_analyzer
>
>/* STACK holds the old VR.  */
>auto_vec > stack;
> +
> +  /* Whether or not to use SCEV to refine ranges.  */
> +  bool use_scev;
>  };
>
>  #endif /* GCC_GIMPLE_SSA_EVRP_ANALYZE_H */
> diff --git a/gcc/gimple-ssa-evrp.c b/gcc/gimple-ssa-evrp.c
> index f56d67f..13e4e16 100644
> --- a/gcc/gimple-ssa-evrp.c
> +++ b/gcc/gimple-ssa-evrp.c
> @@ -69,6 +69,7 @@ class evrp_dom_walker : public dom_walker
>  public:
>evrp_dom_walker ()
>  : dom_walker (CDI_DOMINATORS),
> +  evrp_range_analyzer (true),
>evrp_folder (evrp_range_analyzer.get_vr_values ())
>  {
>need_eh_cleanup = BITMAP_ALLOC (NULL);
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index 838822d..46be749 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -4737,6 +4737,7 @@ insert_range_assertions (void)
>  class vrp_prop : public ssa_propagation_engine
>  {
>   public:
> +  vrp_prop (bool use_scev_) : vr_values (use_scev_) { }
>enum ssa_prop_result visit_stmt (gimple *, edge *, tree *) FINAL OVERRIDE;
>enum ssa_prop_result visit_phi (gphi *) FINAL OVERRIDE;
>
> @@ -6862,7 +6863,7 @@ execute_vrp (bool warn_array_bounds_p)
>/* For visiting PHI nodes we need EDGE_DFS_BACK computed.  */
>mark_dfs_back_edges ();
>
> -  class vrp_prop vrp_prop;
> +  class vrp_prop vrp_prop (true);
>vrp_prop.vrp_initialize ();
>vrp_prop.ssa_propagate ();
>vrp_prop.vrp_finalize (warn_array_bounds_p);
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 2d11861..54b0be5 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -1907,7 +1907,8 @@ vr_values::dump_all_value_ranges (FILE *file)
>
>  /* Initialize VRP lattice.  */
>
> -vr_values::vr_values () : vrp_value_range_pool ("Tree VRP value ranges")
> +vr_values::vr_values (bool use_scev_)
> +  : vrp_value_range_pool ("Tree VRP value ranges"), use_scev (use_scev_)