Re: [PATCH] Simplify conditions in EVRP, handle taken edge

2016-10-24 Thread Jeff Law 

On 10/20/2016 12:10 AM, kugan wrote:

Hi,


On 20/10/16 02:54, Andrew Pinski wrote:

On Wed, Oct 19, 2016 at 1:01 AM, Christophe Lyon
 wrote:

On 18 October 2016 at 09:34, Richard Biener  wrote:

On Mon, 17 Oct 2016, Richard Biener wrote:



This refactors propagation vs. substitution and handles condition
simplification properly as well as passing a known taken edge down
to the DOM walker (avoiding useless work and properly handling PHIs).

If we do all the work it's stupid to not fold away dead code...

Bootstrap and regtest pending on x86_64-unknown-linux-gnu.


The following is what I applied, also fixing a spelling mistake noticed
by Bernhard.


Hi Richard,

This patch is causing regressions on aarch64. These tests now fail:


So I looked into it and found the testcase themselves need to be changed.
The functions are marked as noinline but not noclone.
For an example:
static void __attribute__((noinline))
check_args_8 (int a0, int a1, int a2, int a3, int a4, int a5, int a6,
int a7,
  int a8)




Indeed. In test12 and so on, arguments for check_args_8/check_args_24 is
now becoming constant which enables ipa-cp to create specialized clones.
Though this is good, in order to preserve the tested functionality, we
need to add noclone attribute. Here is a patch to do this.

Regression tested on aatch64-linux-gnu. Is this OK for trunk?

Thanks,
Kugan

gcc/testsuite/ChangeLog:

2016-10-20  Kugan Vivekanandarajah  

* gcc.target/aarch64/test_frame_common.h: Add noclone attribute
such that cloned verions of tested functions are not created.

OK.
jeff

Re: [PATCH] Simplify conditions in EVRP, handle taken edge

2016-10-21 Thread Andrew Pinski 
On Wed, Oct 19, 2016 at 11:10 PM, kugan
 wrote:
> Hi,
>
>
> On 20/10/16 02:54, Andrew Pinski wrote:
>>
>> On Wed, Oct 19, 2016 at 1:01 AM, Christophe Lyon
>>  wrote:
>>>
>>> On 18 October 2016 at 09:34, Richard Biener  wrote:

 On Mon, 17 Oct 2016, Richard Biener wrote:

>
> This refactors propagation vs. substitution and handles condition
> simplification properly as well as passing a known taken edge down
> to the DOM walker (avoiding useless work and properly handling PHIs).
>
> If we do all the work it's stupid to not fold away dead code...
>
> Bootstrap and regtest pending on x86_64-unknown-linux-gnu.


 The following is what I applied, also fixing a spelling mistake noticed
 by Bernhard.

>>> Hi Richard,
>>>
>>> This patch is causing regressions on aarch64. These tests now fail:
>>
>>
>> So I looked into it and found the testcase themselves need to be changed.
>> The functions are marked as noinline but not noclone.
>> For an example:
>> static void __attribute__((noinline))
>> check_args_8 (int a0, int a1, int a2, int a3, int a4, int a5, int a6, int
>> a7,
>>   int a8)
>> 
>>
>
> Indeed. In test12 and so on, arguments for check_args_8/check_args_24 is now
> becoming constant which enables ipa-cp to create specialized clones. Though
> this is good, in order to preserve the tested functionality, we need to add
> noclone attribute. Here is a patch to do this.
>
> Regression tested on aatch64-linux-gnu. Is this OK for trunk?

I think this is obvious, mainly because noinline was already there.

Thanks,
Andrew

>
> Thanks,
> Kugan
>
> gcc/testsuite/ChangeLog:
>
> 2016-10-20  Kugan Vivekanandarajah  
>
> * gcc.target/aarch64/test_frame_common.h: Add noclone attribute
> such that cloned verions of tested functions are not created.
>
>
>
>> Thanks,
>> Andrew
>>
>>>   gcc.target/aarch64/test_frame_12.c scan-assembler-times ldp\tx29,
>>> x30, \\[sp, [0-9]+\\] 1
>>>   gcc.target/aarch64/test_frame_12.c scan-assembler-times sub\tsp, sp,
>>> #[0-9]+ 1
>>>   gcc.target/aarch64/test_frame_15.c scan-assembler-times stp\tx29,
>>> x30, \\[sp, [0-9]+\\] 1
>>>   gcc.target/aarch64/test_frame_15.c scan-assembler-times sub\tsp, sp,
>>> #[0-9]+ 1
>>>   gcc.target/aarch64/test_frame_8.c scan-assembler-times ldr\tx30,
>>> \\[sp, [0-9]+\\] 1
>>>   gcc.target/aarch64/test_frame_8.c scan-assembler-times str\tx30,
>>> \\[sp, [0-9]+\\] 1
>>>
>>> Christophe
>>>
 Richard.

 2016-10-18  Richard Biener  

 * tree-vrp.c (evrp_dom_walker::before_dom_children): Handle
 not visited but non-executable predecessors.  Return taken edge.
 Simplify conditions and refactor propagation vs. folding step.

 * gcc.dg/tree-ssa/pr20318.c: Disable EVRP.
 * gcc.dg/tree-ssa/pr21001.c: Likewise.
 * gcc.dg/tree-ssa/pr21090.c: Likewise.
 * gcc.dg/tree-ssa/pr21294.c: Likewise.
 * gcc.dg/tree-ssa/pr21563.c: Likewise.
 * gcc.dg/tree-ssa/pr23744.c: Likewise.
 * gcc.dg/tree-ssa/pr25382.c: Likewise.
 * gcc.dg/tree-ssa/pr68431.c: Likewise.
 * gcc.dg/tree-ssa/vrp03.c: Likewise.
 * gcc.dg/tree-ssa/vrp06.c: Likewise.
 * gcc.dg/tree-ssa/vrp07.c: Likewise.
 * gcc.dg/tree-ssa/vrp09.c: Likewise.
 * gcc.dg/tree-ssa/vrp19.c: Likewise.
 * gcc.dg/tree-ssa/vrp20.c: Likewise.
 * gcc.dg/tree-ssa/vrp92.c: Likewise.
 * gcc.dg/pr68217.c: Likewise.
 * gcc.dg/predict-9.c: Likewise.
 * gcc.dg/tree-prof/val-prof-5.c: Adjust.
 * gcc.dg/predict-1.c: Likewise.



 Index: gcc/tree-vrp.c
 ===
 --- gcc/tree-vrp.c  (revision 241242)
 +++ gcc/tree-vrp.c  (working copy)
 @@ -10741,12 +10741,13 @@ evrp_dom_walker::before_dom_children (ba
gimple_stmt_iterator gsi;
edge e;
edge_iterator ei;
 -  bool has_unvisived_preds = false;
 +  bool has_unvisited_preds = false;

FOR_EACH_EDGE (e, ei, bb->preds)
 -if (!(e->src->flags & BB_VISITED))
 +if (e->flags & EDGE_EXECUTABLE
 +   && !(e->src->flags & BB_VISITED))
{
 -   has_unvisived_preds = true;
 +   has_unvisited_preds = true;
 break;
}

 @@ -10756,7 +10757,7 @@ evrp_dom_walker::before_dom_children (ba
gphi *phi = gpi.phi ();
tree lhs = PHI_RESULT (phi);
value_range vr_result = VR_INITIALIZER;
 -  if (!has_unvisived_preds
 +  if (!has_unvisited_preds
   && stmt_interesting_for_vrp (phi))
 extract_range_from_phi_node (phi, _result);
else

Re: [PATCH] Simplify conditions in EVRP, handle taken edge

2016-10-20 Thread kugan 

Hi,


On 20/10/16 02:54, Andrew Pinski wrote:

On Wed, Oct 19, 2016 at 1:01 AM, Christophe Lyon
 wrote:

On 18 October 2016 at 09:34, Richard Biener  wrote:

On Mon, 17 Oct 2016, Richard Biener wrote:



This refactors propagation vs. substitution and handles condition
simplification properly as well as passing a known taken edge down
to the DOM walker (avoiding useless work and properly handling PHIs).

If we do all the work it's stupid to not fold away dead code...

Bootstrap and regtest pending on x86_64-unknown-linux-gnu.


The following is what I applied, also fixing a spelling mistake noticed
by Bernhard.


Hi Richard,

This patch is causing regressions on aarch64. These tests now fail:


So I looked into it and found the testcase themselves need to be changed.
The functions are marked as noinline but not noclone.
For an example:
static void __attribute__((noinline))
check_args_8 (int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7,
  int a8)




Indeed. In test12 and so on, arguments for check_args_8/check_args_24 is 
now becoming constant which enables ipa-cp to create specialized clones. 
Though this is good, in order to preserve the tested functionality, we 
need to add noclone attribute. Here is a patch to do this.


Regression tested on aatch64-linux-gnu. Is this OK for trunk?

Thanks,
Kugan

gcc/testsuite/ChangeLog:

2016-10-20  Kugan Vivekanandarajah  

* gcc.target/aarch64/test_frame_common.h: Add noclone attribute
such that cloned verions of tested functions are not created.



Thanks,
Andrew


  gcc.target/aarch64/test_frame_12.c scan-assembler-times ldp\tx29,
x30, \\[sp, [0-9]+\\] 1
  gcc.target/aarch64/test_frame_12.c scan-assembler-times sub\tsp, sp, #[0-9]+ 1
  gcc.target/aarch64/test_frame_15.c scan-assembler-times stp\tx29,
x30, \\[sp, [0-9]+\\] 1
  gcc.target/aarch64/test_frame_15.c scan-assembler-times sub\tsp, sp, #[0-9]+ 1
  gcc.target/aarch64/test_frame_8.c scan-assembler-times ldr\tx30,
\\[sp, [0-9]+\\] 1
  gcc.target/aarch64/test_frame_8.c scan-assembler-times str\tx30,
\\[sp, [0-9]+\\] 1

Christophe


Richard.

2016-10-18  Richard Biener  

* tree-vrp.c (evrp_dom_walker::before_dom_children): Handle
not visited but non-executable predecessors.  Return taken edge.
Simplify conditions and refactor propagation vs. folding step.

* gcc.dg/tree-ssa/pr20318.c: Disable EVRP.
* gcc.dg/tree-ssa/pr21001.c: Likewise.
* gcc.dg/tree-ssa/pr21090.c: Likewise.
* gcc.dg/tree-ssa/pr21294.c: Likewise.
* gcc.dg/tree-ssa/pr21563.c: Likewise.
* gcc.dg/tree-ssa/pr23744.c: Likewise.
* gcc.dg/tree-ssa/pr25382.c: Likewise.
* gcc.dg/tree-ssa/pr68431.c: Likewise.
* gcc.dg/tree-ssa/vrp03.c: Likewise.
* gcc.dg/tree-ssa/vrp06.c: Likewise.
* gcc.dg/tree-ssa/vrp07.c: Likewise.
* gcc.dg/tree-ssa/vrp09.c: Likewise.
* gcc.dg/tree-ssa/vrp19.c: Likewise.
* gcc.dg/tree-ssa/vrp20.c: Likewise.
* gcc.dg/tree-ssa/vrp92.c: Likewise.
* gcc.dg/pr68217.c: Likewise.
* gcc.dg/predict-9.c: Likewise.
* gcc.dg/tree-prof/val-prof-5.c: Adjust.
* gcc.dg/predict-1.c: Likewise.



Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c  (revision 241242)
+++ gcc/tree-vrp.c  (working copy)
@@ -10741,12 +10741,13 @@ evrp_dom_walker::before_dom_children (ba
   gimple_stmt_iterator gsi;
   edge e;
   edge_iterator ei;
-  bool has_unvisived_preds = false;
+  bool has_unvisited_preds = false;

   FOR_EACH_EDGE (e, ei, bb->preds)
-if (!(e->src->flags & BB_VISITED))
+if (e->flags & EDGE_EXECUTABLE
+   && !(e->src->flags & BB_VISITED))
   {
-   has_unvisived_preds = true;
+   has_unvisited_preds = true;
break;
   }

@@ -10756,7 +10757,7 @@ evrp_dom_walker::before_dom_children (ba
   gphi *phi = gpi.phi ();
   tree lhs = PHI_RESULT (phi);
   value_range vr_result = VR_INITIALIZER;
-  if (!has_unvisived_preds
+  if (!has_unvisited_preds
  && stmt_interesting_for_vrp (phi))
extract_range_from_phi_node (phi, _result);
   else
@@ -10764,81 +10765,90 @@ evrp_dom_walker::before_dom_children (ba
   update_value_range (lhs, _result);
 }

+  edge taken_edge = NULL;
+
   /* Visit all other stmts and discover any new VRs possible.  */
   for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next ())
 {
   gimple *stmt = gsi_stmt (gsi);
-  edge taken_edge;
   tree output = NULL_TREE;
   gimple *old_stmt = stmt;
   bool was_noreturn = (is_gimple_call (stmt)
   && gimple_call_noreturn_p (stmt));

-  /* TODO, if found taken_edge, we should visit (return it) and travel
-again to improve VR as done in DOM/SCCVN optimizations.  It should
-be done

Re: [PATCH] Simplify conditions in EVRP, handle taken edge

2016-10-19 Thread Christophe Lyon 
On 18 October 2016 at 09:34, Richard Biener  wrote:
> On Mon, 17 Oct 2016, Richard Biener wrote:
>
>>
>> This refactors propagation vs. substitution and handles condition
>> simplification properly as well as passing a known taken edge down
>> to the DOM walker (avoiding useless work and properly handling PHIs).
>>
>> If we do all the work it's stupid to not fold away dead code...
>>
>> Bootstrap and regtest pending on x86_64-unknown-linux-gnu.
>
> The following is what I applied, also fixing a spelling mistake noticed
> by Bernhard.
>
Hi Richard,

This patch is causing regressions on aarch64. These tests now fail:
  gcc.target/aarch64/test_frame_12.c scan-assembler-times ldp\tx29,
x30, \\[sp, [0-9]+\\] 1
  gcc.target/aarch64/test_frame_12.c scan-assembler-times sub\tsp, sp, #[0-9]+ 1
  gcc.target/aarch64/test_frame_15.c scan-assembler-times stp\tx29,
x30, \\[sp, [0-9]+\\] 1
  gcc.target/aarch64/test_frame_15.c scan-assembler-times sub\tsp, sp, #[0-9]+ 1
  gcc.target/aarch64/test_frame_8.c scan-assembler-times ldr\tx30,
\\[sp, [0-9]+\\] 1
  gcc.target/aarch64/test_frame_8.c scan-assembler-times str\tx30,
\\[sp, [0-9]+\\] 1

Christophe

> Richard.
>
> 2016-10-18  Richard Biener  
>
> * tree-vrp.c (evrp_dom_walker::before_dom_children): Handle
> not visited but non-executable predecessors.  Return taken edge.
> Simplify conditions and refactor propagation vs. folding step.
>
> * gcc.dg/tree-ssa/pr20318.c: Disable EVRP.
> * gcc.dg/tree-ssa/pr21001.c: Likewise.
> * gcc.dg/tree-ssa/pr21090.c: Likewise.
> * gcc.dg/tree-ssa/pr21294.c: Likewise.
> * gcc.dg/tree-ssa/pr21563.c: Likewise.
> * gcc.dg/tree-ssa/pr23744.c: Likewise.
> * gcc.dg/tree-ssa/pr25382.c: Likewise.
> * gcc.dg/tree-ssa/pr68431.c: Likewise.
> * gcc.dg/tree-ssa/vrp03.c: Likewise.
> * gcc.dg/tree-ssa/vrp06.c: Likewise.
> * gcc.dg/tree-ssa/vrp07.c: Likewise.
> * gcc.dg/tree-ssa/vrp09.c: Likewise.
> * gcc.dg/tree-ssa/vrp19.c: Likewise.
> * gcc.dg/tree-ssa/vrp20.c: Likewise.
> * gcc.dg/tree-ssa/vrp92.c: Likewise.
> * gcc.dg/pr68217.c: Likewise.
> * gcc.dg/predict-9.c: Likewise.
> * gcc.dg/tree-prof/val-prof-5.c: Adjust.
> * gcc.dg/predict-1.c: Likewise.
>
>
>
> Index: gcc/tree-vrp.c
> ===
> --- gcc/tree-vrp.c  (revision 241242)
> +++ gcc/tree-vrp.c  (working copy)
> @@ -10741,12 +10741,13 @@ evrp_dom_walker::before_dom_children (ba
>gimple_stmt_iterator gsi;
>edge e;
>edge_iterator ei;
> -  bool has_unvisived_preds = false;
> +  bool has_unvisited_preds = false;
>
>FOR_EACH_EDGE (e, ei, bb->preds)
> -if (!(e->src->flags & BB_VISITED))
> +if (e->flags & EDGE_EXECUTABLE
> +   && !(e->src->flags & BB_VISITED))
>{
> -   has_unvisived_preds = true;
> +   has_unvisited_preds = true;
> break;
>}
>
> @@ -10756,7 +10757,7 @@ evrp_dom_walker::before_dom_children (ba
>gphi *phi = gpi.phi ();
>tree lhs = PHI_RESULT (phi);
>value_range vr_result = VR_INITIALIZER;
> -  if (!has_unvisived_preds
> +  if (!has_unvisited_preds
>   && stmt_interesting_for_vrp (phi))
> extract_range_from_phi_node (phi, _result);
>else
> @@ -10764,81 +10765,90 @@ evrp_dom_walker::before_dom_children (ba
>update_value_range (lhs, _result);
>  }
>
> +  edge taken_edge = NULL;
> +
>/* Visit all other stmts and discover any new VRs possible.  */
>for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next ())
>  {
>gimple *stmt = gsi_stmt (gsi);
> -  edge taken_edge;
>tree output = NULL_TREE;
>gimple *old_stmt = stmt;
>bool was_noreturn = (is_gimple_call (stmt)
>&& gimple_call_noreturn_p (stmt));
>
> -  /* TODO, if found taken_edge, we should visit (return it) and travel
> -again to improve VR as done in DOM/SCCVN optimizations.  It should
> -be done carefully as stmts might prematurely leave a BB like
> -in EH.  */
> -  if (stmt_interesting_for_vrp (stmt))
> +  if (gcond *cond = dyn_cast  (stmt))
> +   {
> + vrp_visit_cond_stmt (cond, _edge);
> + if (taken_edge)
> +   {
> + if (taken_edge->flags & EDGE_TRUE_VALUE)
> +   gimple_cond_make_true (cond);
> + else if (taken_edge->flags & EDGE_FALSE_VALUE)
> +   gimple_cond_make_false (cond);
> + else
> +   gcc_unreachable ();
> +   }
> +   }
> +  else if (stmt_interesting_for_vrp (stmt))
> {
> + edge taken_edge;
>   value_range vr = VR_INITIALIZER;
>   extract_range_from_stmt (stmt, _edge, , );
>   if (output
>   && (vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE))
> -

Re: [PATCH] Simplify conditions in EVRP, handle taken edge

2016-10-18 Thread Richard Biener 
On Mon, 17 Oct 2016, Richard Biener wrote:

> 
> This refactors propagation vs. substitution and handles condition
> simplification properly as well as passing a known taken edge down
> to the DOM walker (avoiding useless work and properly handling PHIs).
> 
> If we do all the work it's stupid to not fold away dead code...
> 
> Bootstrap and regtest pending on x86_64-unknown-linux-gnu.

The following is what I applied, also fixing a spelling mistake noticed
by Bernhard.

Richard.

2016-10-18  Richard Biener  

* tree-vrp.c (evrp_dom_walker::before_dom_children): Handle
not visited but non-executable predecessors.  Return taken edge.
Simplify conditions and refactor propagation vs. folding step.

* gcc.dg/tree-ssa/pr20318.c: Disable EVRP.
* gcc.dg/tree-ssa/pr21001.c: Likewise.
* gcc.dg/tree-ssa/pr21090.c: Likewise.
* gcc.dg/tree-ssa/pr21294.c: Likewise.
* gcc.dg/tree-ssa/pr21563.c: Likewise.
* gcc.dg/tree-ssa/pr23744.c: Likewise.
* gcc.dg/tree-ssa/pr25382.c: Likewise.
* gcc.dg/tree-ssa/pr68431.c: Likewise.
* gcc.dg/tree-ssa/vrp03.c: Likewise.
* gcc.dg/tree-ssa/vrp06.c: Likewise.
* gcc.dg/tree-ssa/vrp07.c: Likewise.
* gcc.dg/tree-ssa/vrp09.c: Likewise.
* gcc.dg/tree-ssa/vrp19.c: Likewise.
* gcc.dg/tree-ssa/vrp20.c: Likewise.
* gcc.dg/tree-ssa/vrp92.c: Likewise.
* gcc.dg/pr68217.c: Likewise.
* gcc.dg/predict-9.c: Likewise.
* gcc.dg/tree-prof/val-prof-5.c: Adjust.
* gcc.dg/predict-1.c: Likewise.



Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c  (revision 241242)
+++ gcc/tree-vrp.c  (working copy)
@@ -10741,12 +10741,13 @@ evrp_dom_walker::before_dom_children (ba
   gimple_stmt_iterator gsi;
   edge e;
   edge_iterator ei;
-  bool has_unvisived_preds = false;
+  bool has_unvisited_preds = false;
 
   FOR_EACH_EDGE (e, ei, bb->preds)
-if (!(e->src->flags & BB_VISITED))
+if (e->flags & EDGE_EXECUTABLE
+   && !(e->src->flags & BB_VISITED))
   {
-   has_unvisived_preds = true;
+   has_unvisited_preds = true;
break;
   }
 
@@ -10756,7 +10757,7 @@ evrp_dom_walker::before_dom_children (ba
   gphi *phi = gpi.phi ();
   tree lhs = PHI_RESULT (phi);
   value_range vr_result = VR_INITIALIZER;
-  if (!has_unvisived_preds
+  if (!has_unvisited_preds
  && stmt_interesting_for_vrp (phi))
extract_range_from_phi_node (phi, _result);
   else
@@ -10764,81 +10765,90 @@ evrp_dom_walker::before_dom_children (ba
   update_value_range (lhs, _result);
 }
 
+  edge taken_edge = NULL;
+
   /* Visit all other stmts and discover any new VRs possible.  */
   for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next ())
 {
   gimple *stmt = gsi_stmt (gsi);
-  edge taken_edge;
   tree output = NULL_TREE;
   gimple *old_stmt = stmt;
   bool was_noreturn = (is_gimple_call (stmt)
   && gimple_call_noreturn_p (stmt));
 
-  /* TODO, if found taken_edge, we should visit (return it) and travel
-again to improve VR as done in DOM/SCCVN optimizations.  It should
-be done carefully as stmts might prematurely leave a BB like
-in EH.  */
-  if (stmt_interesting_for_vrp (stmt))
+  if (gcond *cond = dyn_cast  (stmt))
+   {
+ vrp_visit_cond_stmt (cond, _edge);
+ if (taken_edge)
+   {
+ if (taken_edge->flags & EDGE_TRUE_VALUE)
+   gimple_cond_make_true (cond);
+ else if (taken_edge->flags & EDGE_FALSE_VALUE)
+   gimple_cond_make_false (cond);
+ else
+   gcc_unreachable ();
+   }
+   }
+  else if (stmt_interesting_for_vrp (stmt))
{
+ edge taken_edge;
  value_range vr = VR_INITIALIZER;
  extract_range_from_stmt (stmt, _edge, , );
  if (output
  && (vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE))
-   update_value_range (output, );
- else
-   set_defs_to_varying (stmt);
-
- /* Try folding stmts with the VR discovered.  */
- bool did_replace
-   = replace_uses_in (stmt,
-  op_with_constant_singleton_value_range);
- if (fold_stmt (, follow_single_use_edges)
- || did_replace)
-   update_stmt (gsi_stmt (gsi));
-
- if (did_replace)
{
- /* If we cleaned up EH information from the statement,
-remove EH edges.  */
- if (maybe_clean_or_replace_eh_stmt (old_stmt, stmt))
-   bitmap_set_bit (need_eh_cleanup, bb->index);
-
- /* If we turned a not noreturn call into a noreturn one
-schedule it for fixup.  */
- if (!was_noreturn
- && is_gimple_call (stmt)
-