Re: Handle GIMPLE NOPs in is_maybe_undefined (PR, tree-optimization/79529).

2017-02-17 Thread Richard Biener 
On Fri, Feb 17, 2017 at 10:55 AM, Martin Liška  wrote:
> On 02/16/2017 12:34 PM, Richard Biener wrote:
>> Yes, we should handle all of the "hidden initialized" cases at
>>
>>   /* A PARM_DECL will not have an SSA_NAME_DEF_STMT.  Parameters
>>  get their initial value from function entry.  */
>>   if (SSA_NAME_VAR (t) && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL)
>> continue;
>>
>> maybe add a predicate for those, like
>>
>>  ssa_defined_default_def_p ()
>>
>> right next to ssa_undefined_value_p and use it from there as well.
>
> Hi.
>
> Done in second version of patch.
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. 
> Firefox w/ -flto and -O3
> works fine.
>
> Ready to be installed?

Ok.

Thanks,
Richard.

> Martin

Re: Handle GIMPLE NOPs in is_maybe_undefined (PR, tree-optimization/79529).

2017-02-17 Thread Martin Liška 
On 02/16/2017 12:34 PM, Richard Biener wrote:
> Yes, we should handle all of the "hidden initialized" cases at
> 
>   /* A PARM_DECL will not have an SSA_NAME_DEF_STMT.  Parameters
>  get their initial value from function entry.  */
>   if (SSA_NAME_VAR (t) && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL)
> continue;
> 
> maybe add a predicate for those, like
> 
>  ssa_defined_default_def_p ()
> 
> right next to ssa_undefined_value_p and use it from there as well.

Hi.

Done in second version of patch.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. 
Firefox w/ -flto and -O3
works fine.

Ready to be installed?
Martin
>From d98ab8fef6d634b73eeca74d11161e3cb7b59776 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 16 Feb 2017 17:07:51 +0100
Subject: [PATCH] Introduce ssa_defined_default_def_p function (PR
 tree-optimization/79529).

gcc/ChangeLog:

2017-02-16  Martin Liska  

	* tree-ssa-loop-unswitch.c (is_maybe_undefined): Use
	ssa_defined_default_def_p to handle cases which are implicitly
	defined.
	* tree-ssa.c (ssa_defined_default_def_p): New function.
	(ssa_undefined_value_p): Use ssa_defined_default_def_p to handle cases
	which are implicitly defined.
	* tree-ssa.h (ssa_defined_default_def_p): Declare.
---
 gcc/tree-ssa-loop-unswitch.c |  4 +---
 gcc/tree-ssa.c   | 26 +++---
 gcc/tree-ssa.h   |  2 ++
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/gcc/tree-ssa-loop-unswitch.c b/gcc/tree-ssa-loop-unswitch.c
index afa04e9d110..1845148666d 100644
--- a/gcc/tree-ssa-loop-unswitch.c
+++ b/gcc/tree-ssa-loop-unswitch.c
@@ -134,9 +134,7 @@ is_maybe_undefined (const tree name, gimple *stmt, struct loop *loop)
   if (ssa_undefined_value_p (t, true))
 	return true;
 
-  /* A PARM_DECL will not have an SSA_NAME_DEF_STMT.  Parameters
-	 get their initial value from function entry.  */
-  if (SSA_NAME_VAR (t) && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL)
+  if (ssa_defined_default_def_p (t))
 	continue;
 
   gimple *def = SSA_NAME_DEF_STMT (t);
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index 28020b003f8..831fd61e15f 100644
--- a/gcc/tree-ssa.c
+++ b/gcc/tree-ssa.c
@@ -1251,27 +1251,39 @@ tree_ssa_strip_useless_type_conversions (tree exp)
   return exp;
 }
 
-
-/* Return true if T, an SSA_NAME, has an undefined value.  PARTIAL is what
-   should be returned if the value is only partially undefined.  */
+/* Return true if T, as SSA_NAME, has an implicit default defined value.  */
 
 bool
-ssa_undefined_value_p (tree t, bool partial)
+ssa_defined_default_def_p (tree t)
 {
-  gimple *def_stmt;
   tree var = SSA_NAME_VAR (t);
 
   if (!var)
 ;
   /* Parameters get their initial value from the function entry.  */
   else if (TREE_CODE (var) == PARM_DECL)
-return false;
+return true;
   /* When returning by reference the return address is actually a hidden
  parameter.  */
   else if (TREE_CODE (var) == RESULT_DECL && DECL_BY_REFERENCE (var))
-return false;
+return true;
   /* Hard register variables get their initial value from the ether.  */
   else if (VAR_P (var) && DECL_HARD_REGISTER (var))
+return true;
+
+  return false;
+}
+
+
+/* Return true if T, an SSA_NAME, has an undefined value.  PARTIAL is what
+   should be returned if the value is only partially undefined.  */
+
+bool
+ssa_undefined_value_p (tree t, bool partial)
+{
+  gimple *def_stmt;
+
+  if (ssa_defined_default_def_p (t))
 return false;
 
   /* The value is undefined iff its definition statement is empty.  */
diff --git a/gcc/tree-ssa.h b/gcc/tree-ssa.h
index 6d16ba9f6a0..c99b5eaee82 100644
--- a/gcc/tree-ssa.h
+++ b/gcc/tree-ssa.h
@@ -50,6 +50,8 @@ extern void delete_tree_ssa (function *);
 extern bool tree_ssa_useless_type_conversion (tree);
 extern tree tree_ssa_strip_useless_type_conversions (tree);
 
+
+extern bool ssa_defined_default_def_p (tree t);
 extern bool ssa_undefined_value_p (tree, bool = true);
 extern bool gimple_uses_undefined_value_p (gimple *);
 extern void execute_update_addresses_taken (void);
-- 
2.11.0

Re: Handle GIMPLE NOPs in is_maybe_undefined (PR, tree-optimization/79529).

2017-02-16 Thread Richard Biener 
On Thu, Feb 16, 2017 at 11:45 AM, Aldy Hernandez  wrote:
> On 02/16/2017 03:46 AM, Martin Liška wrote:
>>
>> On 02/15/2017 05:06 PM, Aldy Hernandez wrote:
>>>
>>> On 02/15/2017 09:49 AM, Martin Liška wrote:

 Hi.

 As mentioned in the PR, gimple nops are wrongly handled in
 is_maybe_undefined function.
 Patch can bootstrap on ppc64le-redhat-linux and survives regression
 tests.
>>>
>>>

   gimple *def = SSA_NAME_DEF_STMT (t);

 +  if (!def || gimple_nop_p (def))
 +return true;
 +
>>>
>>>
>>>
>>> Out of curiosity, what is T?
>>
>>
>> It's a SSA name which belongs to a result_decl:
>>
>> (gdb) p debug_tree(t)
>>  > type > type > needs-constructing BLK
>> size 
>> unit size 
>> align 64 symtab 0 alias set -1 canonical type 0x766b0e70
>> fields  context > 0x766954c0 icu_58>
>> pointer_to_this 
>> reference_to_this  chain > 0x70669428 UnicodeString>>
>> public unsigned DI
>> size 
>> unit size 
>> align 64 symtab 0 alias set -1 structural equality>
>> var 
>> def_stmt GIMPLE_NOP
>> version 1
>> ptr-info 0x7fffcc191000>
>>
>>>
>>> Because we early bail out if ssa_undefined_value_p() right before calling
>>> SSA_NAME_DEF_STMT, and ssa_undefined_value_p() will already bail if
>>> gimple_nop_p.
>>>
>>> So T must be one of:
>>>
>>>
>>>   /* Parameters get their initial value from the function entry.  */
>>>   else if (TREE_CODE (var) == PARM_DECL)
>>> return false;
>>>   /* When returning by reference the return address is actually a hidden
>>>  parameter.  */
>>>   else if (TREE_CODE (var) == RESULT_DECL && DECL_BY_REFERENCE (var))
>>> return false;
>>
>>
>> This return statement is taken.
>>
>>>   /* Hard register variables get their initial value from the ether.  */
>>>   else if (VAR_P (var) && DECL_HARD_REGISTER (var))
>>> return false;
>>>
>>> which I wonder if we should special case in is_maybe_undefined.
>>
>>
>> Do we need a special case in the function?
>
>
> It's up to Richi et al, but IMO we should probably treat this as we do
> PARM_DECL's, since according to the comment in ssa_undefined_value_p,
> returning by reference the return address is actually a hidden parameter.
> And as such, we should 'continue' not return in is_maybe_undefined.  This
> way, we can process the rest of the items in the worklist.
>
> We already handle:
>
>   /* A PARM_DECL will not have an SSA_NAME_DEF_STMT.  Parameters
>  get their initial value from function entry.  */
>   if (SSA_NAME_VAR (t) && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL)
> continue;
>
> I say we should also handle the rest of the things we handle in
> ssa_undefined_value_p:
>
>   /* Parameters get their initial value from the function entry.  */
>   else if (TREE_CODE (var) == PARM_DECL)
> return false;
>   /* When returning by reference the return address is actually a hidden
>  parameter.  */
>   else if (TREE_CODE (var) == RESULT_DECL && DECL_BY_REFERENCE (var))
> return false;
>   /* Hard register variables get their initial value from the ether.  */
>   else if (VAR_P (var) && DECL_HARD_REGISTER (var))
> return false;
>
> ...all doing a "continue", as opposed to a return.
>
> And if we're going to duplicate all that code, might as well abstract it out
> to an inline function.
>
> Also, we should probably still gracefully handle an empty SSA_NAME_DEF_STMT
> as you do in your patch, but with a continue as well.
>
> Richi, do you agree?

Yes, we should handle all of the "hidden initialized" cases at

  /* A PARM_DECL will not have an SSA_NAME_DEF_STMT.  Parameters
 get their initial value from function entry.  */
  if (SSA_NAME_VAR (t) && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL)
continue;

maybe add a predicate for those, like

 ssa_defined_default_def_p ()

right next to ssa_undefined_value_p and use it from there as well.

Richard.

>
> Aldy

Re: Handle GIMPLE NOPs in is_maybe_undefined (PR, tree-optimization/79529).

2017-02-16 Thread Aldy Hernandez 

On 02/16/2017 03:46 AM, Martin Liška wrote:

On 02/15/2017 05:06 PM, Aldy Hernandez wrote:

On 02/15/2017 09:49 AM, Martin Liška wrote:

Hi.

As mentioned in the PR, gimple nops are wrongly handled in is_maybe_undefined 
function.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.




  gimple *def = SSA_NAME_DEF_STMT (t);

+  if (!def || gimple_nop_p (def))
+return true;
+



Out of curiosity, what is T?


It's a SSA name which belongs to a result_decl:

(gdb) p debug_tree(t)
 
unit size 
align 64 symtab 0 alias set -1 canonical type 0x766b0e70 fields 
 context 
pointer_to_this  reference_to_this  chain >
public unsigned DI
size 
unit size 
align 64 symtab 0 alias set -1 structural equality>
var 
def_stmt GIMPLE_NOP
version 1
ptr-info 0x7fffcc191000>



Because we early bail out if ssa_undefined_value_p() right before calling 
SSA_NAME_DEF_STMT, and ssa_undefined_value_p() will already bail if 
gimple_nop_p.

So T must be one of:


  /* Parameters get their initial value from the function entry.  */
  else if (TREE_CODE (var) == PARM_DECL)
return false;
  /* When returning by reference the return address is actually a hidden
 parameter.  */
  else if (TREE_CODE (var) == RESULT_DECL && DECL_BY_REFERENCE (var))
return false;


This return statement is taken.


  /* Hard register variables get their initial value from the ether.  */
  else if (VAR_P (var) && DECL_HARD_REGISTER (var))
return false;

which I wonder if we should special case in is_maybe_undefined.


Do we need a special case in the function?


It's up to Richi et al, but IMO we should probably treat this as we do 
PARM_DECL's, since according to the comment in ssa_undefined_value_p, 
returning by reference the return address is actually a hidden 
parameter.  And as such, we should 'continue' not return in 
is_maybe_undefined.  This way, we can process the rest of the items in 
the worklist.


We already handle:

  /* A PARM_DECL will not have an SSA_NAME_DEF_STMT.  Parameters
 get their initial value from function entry.  */
  if (SSA_NAME_VAR (t) && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL)
continue;

I say we should also handle the rest of the things we handle in 
ssa_undefined_value_p:


  /* Parameters get their initial value from the function entry.  */
  else if (TREE_CODE (var) == PARM_DECL)
return false;
  /* When returning by reference the return address is actually a hidden
 parameter.  */
  else if (TREE_CODE (var) == RESULT_DECL && DECL_BY_REFERENCE (var))
return false;
  /* Hard register variables get their initial value from the ether.  */
  else if (VAR_P (var) && DECL_HARD_REGISTER (var))
return false;

...all doing a "continue", as opposed to a return.

And if we're going to duplicate all that code, might as well abstract it 
out to an inline function.


Also, we should probably still gracefully handle an empty 
SSA_NAME_DEF_STMT as you do in your patch, but with a continue as well.


Richi, do you agree?

Aldy

Re: Handle GIMPLE NOPs in is_maybe_undefined (PR, tree-optimization/79529).

2017-02-16 Thread Martin Liška 
On 02/15/2017 05:06 PM, Aldy Hernandez wrote:
> On 02/15/2017 09:49 AM, Martin Liška wrote:
>> Hi.
>>
>> As mentioned in the PR, gimple nops are wrongly handled in 
>> is_maybe_undefined function.
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
>>
>>   gimple *def = SSA_NAME_DEF_STMT (t);
>>
>> +  if (!def || gimple_nop_p (def))
>> +return true;
>> +
> 
> 
> Out of curiosity, what is T?

It's a SSA name which belongs to a result_decl:

(gdb) p debug_tree(t)
 
unit size 
align 64 symtab 0 alias set -1 canonical type 0x766b0e70 fields 
 context 
pointer_to_this  reference_to_this 
 chain >
public unsigned DI
size 
unit size 
align 64 symtab 0 alias set -1 structural equality>
var 
def_stmt GIMPLE_NOP
version 1
ptr-info 0x7fffcc191000>

> 
> Because we early bail out if ssa_undefined_value_p() right before calling 
> SSA_NAME_DEF_STMT, and ssa_undefined_value_p() will already bail if 
> gimple_nop_p.
> 
> So T must be one of:
> 
> 
>   /* Parameters get their initial value from the function entry.  */
>   else if (TREE_CODE (var) == PARM_DECL)
> return false;
>   /* When returning by reference the return address is actually a hidden
>  parameter.  */
>   else if (TREE_CODE (var) == RESULT_DECL && DECL_BY_REFERENCE (var))
> return false;

This return statement is taken.

>   /* Hard register variables get their initial value from the ether.  */
>   else if (VAR_P (var) && DECL_HARD_REGISTER (var))
> return false;
> 
> which I wonder if we should special case in is_maybe_undefined.

Do we need a special case in the function?

M.

> 
> Aldy

Re: Handle GIMPLE NOPs in is_maybe_undefined (PR, tree-optimization/79529).

2017-02-15 Thread Aldy Hernandez 

On 02/15/2017 09:49 AM, Martin Liška wrote:

Hi.

As mentioned in the PR, gimple nops are wrongly handled in is_maybe_undefined 
function.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.




  gimple *def = SSA_NAME_DEF_STMT (t);

+  if (!def || gimple_nop_p (def))
+   return true;
+



Out of curiosity, what is T?

Because we early bail out if ssa_undefined_value_p() right before 
calling SSA_NAME_DEF_STMT, and ssa_undefined_value_p() will already bail 
if gimple_nop_p.


So T must be one of:


  /* Parameters get their initial value from the function entry.  */
  else if (TREE_CODE (var) == PARM_DECL)
return false;
  /* When returning by reference the return address is actually a hidden
 parameter.  */
  else if (TREE_CODE (var) == RESULT_DECL && DECL_BY_REFERENCE (var))
return false;
  /* Hard register variables get their initial value from the ether.  */
  else if (VAR_P (var) && DECL_HARD_REGISTER (var))
return false;

which I wonder if we should special case in is_maybe_undefined.

Aldy

Handle GIMPLE NOPs in is_maybe_undefined (PR, tree-optimization/79529).

2017-02-15 Thread Martin Liška 
Hi.

As mentioned in the PR, gimple nops are wrongly handled in is_maybe_undefined 
function.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin
>From 54b98e2d035f92ec20bf7b548f90b1d00c4c597b Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Wed, 15 Feb 2017 13:46:38 +0100
Subject: [PATCH] Handle GIMPLE NOPs in is_maybe_undefined (PR
 tree-optimization/79529).

gcc/ChangeLog:

2017-02-15  Martin Liska  <mli...@suse.cz>

	PR tree-optimization/79529
	* tree-ssa-loop-unswitch.c (is_maybe_undefined): Bail out when
	spotting a gimple NOP.
---
 gcc/tree-ssa-loop-unswitch.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/tree-ssa-loop-unswitch.c b/gcc/tree-ssa-loop-unswitch.c
index 4ef3a6bf80a..a52e4719bec 100644
--- a/gcc/tree-ssa-loop-unswitch.c
+++ b/gcc/tree-ssa-loop-unswitch.c
@@ -141,6 +141,9 @@ is_maybe_undefined (const tree name, gimple *stmt, struct loop *loop)
 
   gimple *def = SSA_NAME_DEF_STMT (t);
 
+  if (!def || gimple_nop_p (def))
+	return true;
+
   /* Check that all the PHI args are fully defined.  */
   if (gphi *phi = dyn_cast  (def))
 	{
-- 
2.11.0