Re: RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-17 Thread Richard Biener 
On Thu, May 17, 2018 at 3:25 PM Andreas Schwab  wrote:

> On Mai 17 2018, Jason Merrill  wrote:

> > On Thu, May 17, 2018 at 4:14 AM, Andreas Schwab  wrote:
> >> On Mai 16 2018, Andreas Schwab  wrote:
> >>> On Mai 15 2018, Jason Merrill  wrote:
> >>>
>  commit 648ffd02e23ac2695de04ab266b4f8862df6c2ed
>  Author: Jason Merrill 
>  Date:   Tue May 15 20:46:54 2018 -0400
> 
>  * cp-tree.h (cp_expr): Remove copy constructor.
> 
>  * mangle.c (struct releasing_vec): Declare copy
constructor.
> >>>
> >>> I'm getting an ICE on ia64 during the stage1 build of libstdc++
(perhaps
> >>> related that this uses gcc 4.8 as the bootstrap compiler):
> >>
> >> I have now switched to gcc 5 as the bootstrap compiler, which doesn't
> >> have this issue.
> >
> > Aha.  Is it the cp_expr change that confused 4.8?

> I haven't looked closer.

I have no problems with GCC 4.8 on trunk x86_64.

Richard.

> Andreas.

> --
> Andreas Schwab, SUSE Labs, sch...@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Re: RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-17 Thread Andreas Schwab 
On Mai 17 2018, Jason Merrill  wrote:

> On Thu, May 17, 2018 at 4:14 AM, Andreas Schwab  wrote:
>> On Mai 16 2018, Andreas Schwab  wrote:
>>> On Mai 15 2018, Jason Merrill  wrote:
>>>
 commit 648ffd02e23ac2695de04ab266b4f8862df6c2ed
 Author: Jason Merrill 
 Date:   Tue May 15 20:46:54 2018 -0400

 * cp-tree.h (cp_expr): Remove copy constructor.

 * mangle.c (struct releasing_vec): Declare copy constructor.
>>>
>>> I'm getting an ICE on ia64 during the stage1 build of libstdc++ (perhaps
>>> related that this uses gcc 4.8 as the bootstrap compiler):
>>
>> I have now switched to gcc 5 as the bootstrap compiler, which doesn't
>> have this issue.
>
> Aha.  Is it the cp_expr change that confused 4.8?

I haven't looked closer.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-17 Thread Jason Merrill 
On Thu, May 17, 2018 at 4:14 AM, Andreas Schwab  wrote:
> On Mai 16 2018, Andreas Schwab  wrote:
>> On Mai 15 2018, Jason Merrill  wrote:
>>
>>> commit 648ffd02e23ac2695de04ab266b4f8862df6c2ed
>>> Author: Jason Merrill 
>>> Date:   Tue May 15 20:46:54 2018 -0400
>>>
>>> * cp-tree.h (cp_expr): Remove copy constructor.
>>>
>>> * mangle.c (struct releasing_vec): Declare copy constructor.
>>
>> I'm getting an ICE on ia64 during the stage1 build of libstdc++ (perhaps
>> related that this uses gcc 4.8 as the bootstrap compiler):
>
> I have now switched to gcc 5 as the bootstrap compiler, which doesn't
> have this issue.

Aha.  Is it the cp_expr change that confused 4.8?

Jason

Re: RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-17 Thread Andreas Schwab 
On Mai 16 2018, Andreas Schwab  wrote:

> On Mai 15 2018, Jason Merrill  wrote:
>
>> commit 648ffd02e23ac2695de04ab266b4f8862df6c2ed
>> Author: Jason Merrill 
>> Date:   Tue May 15 20:46:54 2018 -0400
>>
>> * cp-tree.h (cp_expr): Remove copy constructor.
>> 
>> * mangle.c (struct releasing_vec): Declare copy constructor.
>
> I'm getting an ICE on ia64 during the stage1 build of libstdc++ (perhaps
> related that this uses gcc 4.8 as the bootstrap compiler):

I have now switched to gcc 5 as the bootstrap compiler, which doesn't
have this issue.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-16 Thread Martin Jambor 
Hi,

On Wed, May 16 2018, Jason Merrill wrote:
> On Wed, May 16, 2018 at 6:35 AM, Martin Jambor  wrote:
>> On Tue, May 15 2018, Jason Merrill wrote:
>>> In C++11 and up, the implicitly-declared copy constructor and
>>> assignment operator are deprecated if one of them, or the destructor,
>>> is user-provided.  Implementing that in G++ turned up a few dodgy uses
>>> in the compiler.
>>>
>>> In general it's unsafe to copy an ipa_edge_args, because if one of the
>>> pointers is non-null you get two copies of a vec pointer, and when one
>>> of the objects is destroyed it frees the vec and leaves the other
>>> object pointing to freed memory.  This specific example is safe
>>> because it only copies from an object with null pointers, but it would
>>> be better to avoid the copy.  OK for trunk?
>>
>> I have had a look and found out that the function in question
>> (ipa_free_edge_args_substructures) has no uses, apparently I forgot to
>> remove it when I did the conversion of jump functions to be stored in
>> call graph edge summaries.  So thanks lot for spotting this but I'd
>> prefer the following (compiled but untested) patch:
>>
>> Martin
>>
>>
>> 2018-05-16  Martin Jambor  
>>
>> * ipa-prop.c (ipa_free_all_edge_args): Remove.
>> * ipa-prop.h (ipa_free_all_edge_args): Likewise.
>
> That works for me, too.
>
> Jason


OK, so after bootstrapping and testing on x86_64, I committed the patch
as obvious.

Thanks,

Martin


2018-05-16  Martin Jambor  

* ipa-prop.c (ipa_free_all_edge_args): Remove.
* ipa-prop.h (ipa_free_all_edge_args): Likewise.
---
 gcc/ipa-prop.c | 10 --
 gcc/ipa-prop.h |  1 -
 2 files changed, 11 deletions(-)

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 38441cc49bc..19d55cda009 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3708,16 +3708,6 @@ ipa_check_create_edge_args (void)
 ipa_vr_hash_table = hash_table::create_ggc (37);
 }
 
-/* Frees all dynamically allocated structures that the argument info points
-   to.  */
-
-void
-ipa_free_edge_args_substructures (struct ipa_edge_args *args)
-{
-  vec_free (args->jump_functions);
-  *args = ipa_edge_args ();
-}
-
 /* Free all ipa_edge structures.  */
 
 void
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index a61e06135e3..dc45cea9c71 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -664,7 +664,6 @@ extern GTY(()) vec 
*ipcp_transformations;
 void ipa_create_all_node_params (void);
 void ipa_create_all_edge_args (void);
 void ipa_check_create_edge_args (void);
-void ipa_free_edge_args_substructures (struct ipa_edge_args *);
 void ipa_free_all_node_params (void);
 void ipa_free_all_edge_args (void);
 void ipa_free_all_structures_after_ipa_cp (void);
-- 
2.16.3

Re: RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-16 Thread Jason Merrill 
On Wed, May 16, 2018 at 6:35 AM, Martin Jambor  wrote:
> On Tue, May 15 2018, Jason Merrill wrote:
>> In C++11 and up, the implicitly-declared copy constructor and
>> assignment operator are deprecated if one of them, or the destructor,
>> is user-provided.  Implementing that in G++ turned up a few dodgy uses
>> in the compiler.
>>
>> In general it's unsafe to copy an ipa_edge_args, because if one of the
>> pointers is non-null you get two copies of a vec pointer, and when one
>> of the objects is destroyed it frees the vec and leaves the other
>> object pointing to freed memory.  This specific example is safe
>> because it only copies from an object with null pointers, but it would
>> be better to avoid the copy.  OK for trunk?
>
> I have had a look and found out that the function in question
> (ipa_free_edge_args_substructures) has no uses, apparently I forgot to
> remove it when I did the conversion of jump functions to be stored in
> call graph edge summaries.  So thanks lot for spotting this but I'd
> prefer the following (compiled but untested) patch:
>
> Martin
>
>
> 2018-05-16  Martin Jambor  
>
> * ipa-prop.c (ipa_free_all_edge_args): Remove.
> * ipa-prop.h (ipa_free_all_edge_args): Likewise.

That works for me, too.

Jason

Re: RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-16 Thread Martin Jambor 
Hi,

On Tue, May 15 2018, Jason Merrill wrote:
> In C++11 and up, the implicitly-declared copy constructor and
> assignment operator are deprecated if one of them, or the destructor,
> is user-provided.  Implementing that in G++ turned up a few dodgy uses
> in the compiler.
>
> In general it's unsafe to copy an ipa_edge_args, because if one of the
> pointers is non-null you get two copies of a vec pointer, and when one
> of the objects is destroyed it frees the vec and leaves the other
> object pointing to freed memory.  This specific example is safe
> because it only copies from an object with null pointers, but it would
> be better to avoid the copy.  OK for trunk?

I have had a look and found out that the function in question
(ipa_free_edge_args_substructures) has no uses, apparently I forgot to
remove it when I did the conversion of jump functions to be stored in
call graph edge summaries.  So thanks lot for spotting this but I'd
prefer the following (compiled but untested) patch:

Martin


2018-05-16  Martin Jambor  

* ipa-prop.c (ipa_free_all_edge_args): Remove.
* ipa-prop.h (ipa_free_all_edge_args): Likewise.


diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 38441cc49bc..19d55cda009 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3708,16 +3708,6 @@ ipa_check_create_edge_args (void)
 ipa_vr_hash_table = hash_table::create_ggc (37);
 }
 
-/* Frees all dynamically allocated structures that the argument info points
-   to.  */
-
-void
-ipa_free_edge_args_substructures (struct ipa_edge_args *args)
-{
-  vec_free (args->jump_functions);
-  *args = ipa_edge_args ();
-}
-
 /* Free all ipa_edge structures.  */
 
 void
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index a61e06135e3..dc45cea9c71 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -664,7 +664,6 @@ extern GTY(()) vec 
*ipcp_transformations;
 void ipa_create_all_node_params (void);
 void ipa_create_all_edge_args (void);
 void ipa_check_create_edge_args (void);
-void ipa_free_edge_args_substructures (struct ipa_edge_args *);
 void ipa_free_all_node_params (void);
 void ipa_free_all_edge_args (void);
 void ipa_free_all_structures_after_ipa_cp (void);

Re: RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-16 Thread Andreas Schwab 
On Mai 15 2018, Jason Merrill  wrote:

> commit 648ffd02e23ac2695de04ab266b4f8862df6c2ed
> Author: Jason Merrill 
> Date:   Tue May 15 20:46:54 2018 -0400
>
> * cp-tree.h (cp_expr): Remove copy constructor.
> 
> * mangle.c (struct releasing_vec): Declare copy constructor.

I'm getting an ICE on ia64 during the stage1 build of libstdc++ (perhaps
related that this uses gcc 4.8 as the bootstrap compiler):

/usr/local/gcc/gcc-20180516/Build/./gcc/xgcc -shared-libgcc 
-B/usr/local/gcc/gcc-20180516/Build/./gcc -nostdinc++ 
-L/usr/local/gcc/gcc-20180516/Build/ia64-suse-linux/libstdc++-v3/src 
-L/usr/local/gcc/gcc-20180516/Build/ia64-suse-linux/libstdc++-v3/src/.libs 
-L/usr/local/gcc/gcc-20180516/Build/ia64-suse-linux/libstdc++-v3/libsupc++/.libs
 -B/usr/ia64-suse-linux/bin/ -B/usr/ia64-suse-linux/lib/ -isystem 
/usr/ia64-suse-linux/include -isystem /usr/ia64-suse-linux/sys-include   
-fno-checking -x c++-header -nostdinc++ -O2 -g -D_GNU_SOURCE  
-I/usr/local/gcc/gcc-20180516/Build/ia64-suse-linux/libstdc++-v3/include/ia64-suse-linux
 -I/usr/local/gcc/gcc-20180516/Build/ia64-suse-linux/libstdc++-v3/include 
-I/usr/local/gcc/gcc-20180516/libstdc++-v3/libsupc++  -O2 -g -std=gnu++0x 
/usr/local/gcc/gcc-20180516/libstdc++-v3/include/precompiled/stdc++.h \
-o ia64-suse-linux/bits/stdc++.h.gch/O2ggnu++0x.gch
In file included from 
/usr/local/gcc/gcc-20180516/Build/ia64-suse-linux/libstdc++-v3/include/cmath:42,
 from 
/usr/local/gcc/gcc-20180516/libstdc++-v3/include/precompiled/stdc++.h:41:
/usr/local/gcc/gcc-20180516/Build/ia64-suse-linux/libstdc++-v3/include/bits/cpp_type_traits.h:89:34:
 internal compiler error: in cp_parser_lookup_name, at cp/parser.c:26107
   enum { __value = bool(_Sp::__value) || bool(_Tp::__value) };
  ^~~
0x403a5cff cp_parser_lookup_name
../../gcc/cp/parser.c:26107
0x403d576f cp_parser_primary_expression
../../gcc/cp/parser.c:5517
0x403f1daf cp_parser_postfix_expression
../../gcc/cp/parser.c:7008
0x4040534f cp_parser_unary_expression
../../gcc/cp/parser.c:8300
0x403b2abf cp_parser_cast_expression
../../gcc/cp/parser.c:9068
0x403b3f9f cp_parser_binary_expression
../../gcc/cp/parser.c:9170
0x403b518f cp_parser_assignment_expression
../../gcc/cp/parser.c:9466
0x403bbf9f cp_parser_parenthesized_expression_list
../../gcc/cp/parser.c:7740
0x403bde1f cp_parser_functional_cast
../../gcc/cp/parser.c:27387
0x403f1c9f cp_parser_postfix_expression
../../gcc/cp/parser.c:6931
0x4040534f cp_parser_unary_expression
../../gcc/cp/parser.c:8300
0x403b2abf cp_parser_cast_expression
../../gcc/cp/parser.c:9068
0x403b3f9f cp_parser_binary_expression
../../gcc/cp/parser.c:9170
0x403b518f cp_parser_assignment_expression
../../gcc/cp/parser.c:9466
0x403b775f cp_parser_constant_expression
../../gcc/cp/parser.c:9748
0x403ce8bf cp_parser_enumerator_definition
../../gcc/cp/parser.c:18410
0x403ce8bf cp_parser_enumerator_list
../../gcc/cp/parser.c:18350
0x403ce8bf cp_parser_enum_specifier
../../gcc/cp/parser.c:18277
0x403ce8bf cp_parser_type_specifier
../../gcc/cp/parser.c:16736
0x403fad7f cp_parser_decl_specifier_seq
../../gcc/cp/parser.c:13606

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-16 Thread Richard Biener 
On Wed, May 16, 2018 at 2:58 AM Jason Merrill  wrote:

> In C++11 and up, the implicitly-declared copy constructor and
> assignment operator are deprecated if one of them, or the destructor,
> is user-provided.  Implementing that in G++ turned up a few dodgy uses
> in the compiler.

> In general it's unsafe to copy an ipa_edge_args, because if one of the
> pointers is non-null you get two copies of a vec pointer, and when one
> of the objects is destroyed it frees the vec and leaves the other
> object pointing to freed memory.  This specific example is safe
> because it only copies from an object with null pointers, but it would
> be better to avoid the copy.  OK for trunk?

> It's unsafe to copy a releasing_vec for the same reason.  There are a
> few places where the copy constructor is nominally used to initialize
> a releasing_vec variable from a temporary returned from a function; in
> these cases no actual copy is done, and the function directly
> initializes the variable, so it's safe.  I made this clearer by
> declaring the copy constructor but not defining it, so uses that get
> elided are accepted, but uses that actually want to copy will fail to
> link.

> In cp_expr we defined the copy constructor to do the same thing that
> the implicit definition would do, causing the copy assignment operator
> to be deprecated.  We don't need the copy constructor, so let's remove
> it.

> Tested x86_64-pc-linux-gnu.  Are the ipa-prop bits OK for trunk?

Yes.

Richard.

RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=

2018-05-15 Thread Jason Merrill 
In C++11 and up, the implicitly-declared copy constructor and
assignment operator are deprecated if one of them, or the destructor,
is user-provided.  Implementing that in G++ turned up a few dodgy uses
in the compiler.

In general it's unsafe to copy an ipa_edge_args, because if one of the
pointers is non-null you get two copies of a vec pointer, and when one
of the objects is destroyed it frees the vec and leaves the other
object pointing to freed memory.  This specific example is safe
because it only copies from an object with null pointers, but it would
be better to avoid the copy.  OK for trunk?

It's unsafe to copy a releasing_vec for the same reason.  There are a
few places where the copy constructor is nominally used to initialize
a releasing_vec variable from a temporary returned from a function; in
these cases no actual copy is done, and the function directly
initializes the variable, so it's safe.  I made this clearer by
declaring the copy constructor but not defining it, so uses that get
elided are accepted, but uses that actually want to copy will fail to
link.

In cp_expr we defined the copy constructor to do the same thing that
the implicit definition would do, causing the copy assignment operator
to be deprecated.  We don't need the copy constructor, so let's remove
it.

Tested x86_64-pc-linux-gnu.  Are the ipa-prop bits OK for trunk?
commit 560b104324b10814dcaaf65606943d4ca55647d6
Author: Jason Merrill 
Date:   Tue May 15 20:47:41 2018 -0400

* ipa-prop.h (ipa_edge_args::release): Factor out of destructor.

* ipa-prop.c (ipa_free_edge_args_substructures): Use it.

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 38441cc49bc..11bbf356ce1 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3714,8 +3714,7 @@ ipa_check_create_edge_args (void)
 void
 ipa_free_edge_args_substructures (struct ipa_edge_args *args)
 {
-  vec_free (args->jump_functions);
-  *args = ipa_edge_args ();
+  args->release ();
 }
 
 /* Free all ipa_edge structures.  */
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index a61e06135e3..08919acb46c 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -569,13 +569,18 @@ class GTY((for_user)) ipa_edge_args
   ipa_edge_args () : jump_functions (NULL), polymorphic_call_contexts (NULL)
 {}
 
-  /* Destructor.  */
-  ~ipa_edge_args ()
+  void release ()
 {
   vec_free (jump_functions);
   vec_free (polymorphic_call_contexts);
 }
 
+  /* Destructor.  */
+  ~ipa_edge_args ()
+{
+  release ();
+}
+
   /* Vectors of the callsite's jump function and polymorphic context
  information of each parameter.  */
   vec *jump_functions;
commit 648ffd02e23ac2695de04ab266b4f8862df6c2ed
Author: Jason Merrill 
Date:   Tue May 15 20:46:54 2018 -0400

* cp-tree.h (cp_expr): Remove copy constructor.

* mangle.c (struct releasing_vec): Declare copy constructor.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 9a2eb3be4d1..cab926028b8 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -59,9 +59,6 @@ public:
   cp_expr (tree value, location_t loc):
 m_value (value), m_loc (loc) {}
 
-  cp_expr (const cp_expr ) :
-m_value (other.m_value), m_loc (other.m_loc) {}
-
   /* Implicit conversions to tree.  */
   operator tree () const { return m_value; }
   tree & operator* () { return m_value; }
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 6a7df804caf..59a3111fba2 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -1555,6 +1555,10 @@ struct releasing_vec
   releasing_vec (vec_t *v): v(v) { }
   releasing_vec (): v(make_tree_vector ()) { }
 
+  /* Copy constructor is deliberately declared but not defined,
+ copies must always be elided.  */
+  releasing_vec (const releasing_vec &);
+
   vec_t * () const { return *v; }
   vec_t *operator-> () const { return v; }
   vec_t *get () const { return v; }