diff mbox series

Fix PR83033

Message ID gkr36n6t055.fsf@arm.com
State New
Headers show
Series Fix PR83033 | expand

Commit Message

Andrea Corallo March 29, 2019, 11:01 a.m. UTC
Hi all,
simple patch addressing minor style issue into gcc/config/aarch64/cortex-a57-fma-steering.c.

make BOOT_CFLAGS='-mcpu=cortex-a57' bootstrap

Okay for trunk?

Bests
  Andrea


2019-03-29  Andrea Corallo  <andrea.corallo@arm.com>

	PR target/83033
	* config/aarch64/cortex-a57-fma-steering.c
	(fma_forest): Fix missing copy constructor.
	(fma_root_node): Likewise.
	(func_fma_steering): Likewise.

Comments

Kyrill Tkachov March 29, 2019, 12:10 p.m. UTC | #1
Hi Alejandro,

On 3/29/19 11:01 AM, Andrea Corallo wrote:
> Hi all,
> simple patch addressing minor style issue into 
> gcc/config/aarch64/cortex-a57-fma-steering.c.
>
> make BOOT_CFLAGS='-mcpu=cortex-a57' bootstrap
>
> Okay for trunk?
>
> Bests
>   Andrea
>
Thanks for the patch.

This looks ok to me but you'll need approval from an aarch64 maintainer 
(I've CC'ed them for you)

Kyrill


>
> 2019-03-29  Andrea Corallo  <andrea.corallo@arm.com>
>
>         PR target/83033
>         * config/aarch64/cortex-a57-fma-steering.c
>         (fma_forest): Fix missing copy constructor.
>         (fma_root_node): Likewise.
>         (func_fma_steering): Likewise.
>
Kyrill Tkachov March 29, 2019, 2:08 p.m. UTC | #2
On 3/29/19 12:10 PM, Kyrill Tkachov wrote:
> Hi Alejandro,
>
Sorry, I meant to say Andrea!

Kyrill

> On 3/29/19 11:01 AM, Andrea Corallo wrote:
> > Hi all,
> > simple patch addressing minor style issue into
> > gcc/config/aarch64/cortex-a57-fma-steering.c.
> >
> > make BOOT_CFLAGS='-mcpu=cortex-a57' bootstrap
> >
> > Okay for trunk?
> >
> > Bests
> >   Andrea
> >
> Thanks for the patch.
>
> This looks ok to me but you'll need approval from an aarch64 maintainer
> (I've CC'ed them for you)
>
> Kyrill
>
>
> >
> > 2019-03-29  Andrea Corallo <andrea.corallo@arm.com>
> >
> >         PR target/83033
> >         * config/aarch64/cortex-a57-fma-steering.c
> >         (fma_forest): Fix missing copy constructor.
> >         (fma_root_node): Likewise.
> >         (func_fma_steering): Likewise.
> >
Richard Earnshaw (lists) April 4, 2019, 4:53 p.m. UTC | #3
On 29/03/2019 11:01, Andrea Corallo wrote:
> Hi all,
> simple patch addressing minor style issue into gcc/config/aarch64/cortex-a57-fma-steering.c.
> 
> make BOOT_CFLAGS='-mcpu=cortex-a57' bootstrap
> 
> Okay for trunk?
> 
> Bests
>   Andrea
> 
> 
> 2019-03-29  Andrea Corallo  <andrea.corallo@arm.com>
> 
> 	PR target/83033
> 	* config/aarch64/cortex-a57-fma-steering.c
> 	(fma_forest): Fix missing copy constructor.
> 	(fma_root_node): Likewise.
> 	(func_fma_steering): Likewise.
> 

These should be commented, even if it's as simple as "Prohibit copy
construction."

R.

> 
> 83033.patch
> 
> diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c b/gcc/config/aarch64/cortex-a57-fma-steering.c
> index f2da03a..a390a62 100644
> --- a/gcc/config/aarch64/cortex-a57-fma-steering.c
> +++ b/gcc/config/aarch64/cortex-a57-fma-steering.c
> @@ -114,6 +114,8 @@ public:
>    void dispatch ();
>  
>  private:
> +  fma_forest (const fma_forest &);
> +
>    /* The list of roots that form this forest.  */
>    std::list<fma_root_node *> *m_roots;
>  
> @@ -180,6 +182,8 @@ public:
>    void dump_info (fma_forest *);
>  
>  private:
> +  fma_root_node (const fma_root_node &);
> +
>    /* The forest this node belonged to when it was created.  */
>    fma_forest *m_forest;
>  };
> @@ -203,6 +207,7 @@ public:
>    void execute_fma_steering ();
>  
>  private:
> +  func_fma_steering (const func_fma_steering &);
>    void dfs (void (*) (fma_forest *), void (*) (fma_forest *, fma_root_node *),
>  	    void (*) (fma_forest *, fma_node *), bool);
>    void analyze ();
>
Andrea Corallo April 5, 2019, 3:53 p.m. UTC | #4
Richard Earnshaw (lists) writes:

> On 29/03/2019 11:01, Andrea Corallo wrote:
>> Hi all,
>> simple patch addressing minor style issue into gcc/config/aarch64/cortex-a57-fma-steering.c.
>>
>> make BOOT_CFLAGS='-mcpu=cortex-a57' bootstrap
>>
>> Okay for trunk?
>>
>> Bests
>>   Andrea
>>
>>
>> 2019-03-29  Andrea Corallo  <andrea.corallo@arm.com>
>>
>> 	PR target/83033
>> 	* config/aarch64/cortex-a57-fma-steering.c
>> 	(fma_forest): Fix missing copy constructor.
>> 	(fma_root_node): Likewise.
>> 	(func_fma_steering): Likewise.
>>
>
> These should be commented, even if it's as simple as "Prohibit copy
> construction."
>
> R.

>
>>

Hi Richard,
here we go.

Bests
  Andrea

2019-03-29  Andrea Corallo  <andrea.corallo@arm.com>

	PR target/83033
	* config/aarch64/cortex-a57-fma-steering.c
	(fma_forest): Prohibit copy construction.
	(fma_root_node): Prohibit copy construction.
	(func_fma_steering): Prohibit copy construction.
diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c b/gcc/config/aarch64/cortex-a57-fma-steering.c
index f2da03a..a390a62 100644
--- a/gcc/config/aarch64/cortex-a57-fma-steering.c
+++ b/gcc/config/aarch64/cortex-a57-fma-steering.c
@@ -114,6 +114,8 @@ public:
   void dispatch ();
 
 private:
+  fma_forest (const fma_forest &);
+
   /* The list of roots that form this forest.  */
   std::list<fma_root_node *> *m_roots;
 
@@ -180,6 +182,8 @@ public:
   void dump_info (fma_forest *);
 
 private:
+  fma_root_node (const fma_root_node &);
+
   /* The forest this node belonged to when it was created.  */
   fma_forest *m_forest;
 };
@@ -203,6 +207,7 @@ public:
   void execute_fma_steering ();
 
 private:
+  func_fma_steering (const func_fma_steering &);
   void dfs (void (*) (fma_forest *), void (*) (fma_forest *, fma_root_node *),
 	    void (*) (fma_forest *, fma_node *), bool);
   void analyze ();
Richard Earnshaw (lists) April 5, 2019, 11 p.m. UTC | #5
On 05/04/2019 16:53, Andrea Corallo wrote:
> 
> Richard Earnshaw (lists) writes:
> 
>> On 29/03/2019 11:01, Andrea Corallo wrote:
>>> Hi all,
>>> simple patch addressing minor style issue into gcc/config/aarch64/cortex-a57-fma-steering.c.
>>>
>>> make BOOT_CFLAGS='-mcpu=cortex-a57' bootstrap
>>>
>>> Okay for trunk?
>>>
>>> Bests
>>>   Andrea
>>>
>>>
>>> 2019-03-29  Andrea Corallo  <andrea.corallo@arm.com>
>>>
>>> 	PR target/83033
>>> 	* config/aarch64/cortex-a57-fma-steering.c
>>> 	(fma_forest): Fix missing copy constructor.
>>> 	(fma_root_node): Likewise.
>>> 	(func_fma_steering): Likewise.
>>>
>>
>> These should be commented, even if it's as simple as "Prohibit copy
>> construction."
>>
>> R.
> 
>>
>>>
> 
> Hi Richard,
> here we go.
> 

Really?  I don't see any change since the last version... :-(

R.

> Bests
>   Andrea
> 
> 2019-03-29  Andrea Corallo  <andrea.corallo@arm.com>
> 
> 	PR target/83033
> 	* config/aarch64/cortex-a57-fma-steering.c
> 	(fma_forest): Prohibit copy construction.
> 	(fma_root_node): Prohibit copy construction.
> 	(func_fma_steering): Prohibit copy construction.
>
Richard Earnshaw (lists) April 6, 2019, 11 a.m. UTC | #6
On 06/04/2019 00:00, Richard Earnshaw (lists) wrote:
> On 05/04/2019 16:53, Andrea Corallo wrote:
>>
>> Richard Earnshaw (lists) writes:
>>
>>> On 29/03/2019 11:01, Andrea Corallo wrote:
>>>> Hi all,
>>>> simple patch addressing minor style issue into gcc/config/aarch64/cortex-a57-fma-steering.c.
>>>>
>>>> make BOOT_CFLAGS='-mcpu=cortex-a57' bootstrap
>>>>
>>>> Okay for trunk?
>>>>
>>>> Bests
>>>>   Andrea
>>>>
>>>>
>>>> 2019-03-29  Andrea Corallo  <andrea.corallo@arm.com>
>>>>
>>>> 	PR target/83033
>>>> 	* config/aarch64/cortex-a57-fma-steering.c
>>>> 	(fma_forest): Fix missing copy constructor.
>>>> 	(fma_root_node): Likewise.
>>>> 	(func_fma_steering): Likewise.
>>>>
>>>
>>> These should be commented, even if it's as simple as "Prohibit copy
>>> construction."
>>>
>>> R.
>>
>>>
>>>>
>>
>> Hi Richard,
>> here we go.
>>
> 
> Really?  I don't see any change since the last version... :-(
> 

Ah, you've just changed the ChangeLog entries.  By comments, I meant in
the source code, so that it's clear these don't exist.  ChangeLog update
is good too.

R.

> R.
> 
>> Bests
>>   Andrea
>>
>> 2019-03-29  Andrea Corallo  <andrea.corallo@arm.com>
>>
>> 	PR target/83033
>> 	* config/aarch64/cortex-a57-fma-steering.c
>> 	(fma_forest): Prohibit copy construction.
>> 	(fma_root_node): Prohibit copy construction.
>> 	(func_fma_steering): Prohibit copy construction.
>>
>
Andrea Corallo April 8, 2019, 8:59 a.m. UTC | #7
Richard Earnshaw (lists) writes:

> Ah, you've just changed the ChangeLog entries.  By comments, I meant in
> the source code, so that it's clear these don't exist.  ChangeLog update
> is good too.
>
> R.

Hi Richard,
sorry my misunderstanding.

Bests
  Andrea


2019-03-29  Andrea Corallo  andrea.corallo@arm.com

PR target/83033
* config/aarch64/cortex-a57-fma-steering.c
(fma_forest): Prohibit copy construction.
(fma_root_node): Prohibit copy construction.
(func_fma_steering): Prohibit copy construction.
From 372ffe36fd0d59aa3c0ba1f8794c7aff9cee84a6 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Thu, 28 Mar 2019 23:17:17 +0100
Subject: [PATCH] imporove cortex-a57-fma-steering.c

---
 gcc/config/aarch64/cortex-a57-fma-steering.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c b/gcc/config/aarch64/cortex-a57-fma-steering.c
index f2da03a..4792d1f 100644
--- a/gcc/config/aarch64/cortex-a57-fma-steering.c
+++ b/gcc/config/aarch64/cortex-a57-fma-steering.c
@@ -114,6 +114,9 @@ public:
   void dispatch ();
 
 private:
+  /* Prohibit copy construction.  */
+  fma_forest (const fma_forest &);
+
   /* The list of roots that form this forest.  */
   std::list<fma_root_node *> *m_roots;
 
@@ -147,6 +150,9 @@ public:
   void set_head (du_head *);
   void rename (fma_forest *);
   void dump_info (fma_forest *);
+private:
+  /* Prohibit copy construction.  */
+  fma_node (const fma_node &);
 
 protected:
   /* Root node that lead to this node.  */
@@ -203,6 +209,9 @@ public:
   void execute_fma_steering ();
 
 private:
+  /* Prohibit copy construction.  */
+  func_fma_steering (const func_fma_steering &);
+
   void dfs (void (*) (fma_forest *), void (*) (fma_forest *, fma_root_node *),
 	    void (*) (fma_forest *, fma_node *), bool);
   void analyze ();
Richard Earnshaw (lists) April 8, 2019, 2:04 p.m. UTC | #8
On 08/04/2019 09:59, Andrea Corallo wrote:
> 
> Richard Earnshaw (lists) writes:
> 
>> Ah, you've just changed the ChangeLog entries.  By comments, I meant in
>> the source code, so that it's clear these don't exist.  ChangeLog update
>> is good too.
>>
>> R.
> 
> Hi Richard,
> sorry my misunderstanding.
> 

NP.  I've put this in.  For future reference, I've made a few very minor
tweaks before applying...

> Bests
>   Andrea
> 
> 
> 2019-03-29  Andrea Corallo  andrea.corallo@arm.com

email address should be enclosed in angle brackets <name@addr>.

> 
> PR target/83033
> * config/aarch64/cortex-a57-fma-steering.c
> (fma_forest): Prohibit copy construction.
> (fma_root_node): Prohibit copy construction.

Generally speaking we use 'Likewise' when the same change is made to
multiple functions.

> (func_fma_steering): Prohibit copy construction.
> 
> 
> PR83033.patch
> 
> From 372ffe36fd0d59aa3c0ba1f8794c7aff9cee84a6 Mon Sep 17 00:00:00 2001
> From: Andrea Corallo <andrea.corallo@arm.com>
> Date: Thu, 28 Mar 2019 23:17:17 +0100
> Subject: [PATCH] imporove cortex-a57-fma-steering.c
> 
> ---
>  gcc/config/aarch64/cortex-a57-fma-steering.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c b/gcc/config/aarch64/cortex-a57-fma-steering.c
> index f2da03a..4792d1f 100644
> --- a/gcc/config/aarch64/cortex-a57-fma-steering.c
> +++ b/gcc/config/aarch64/cortex-a57-fma-steering.c
> @@ -114,6 +114,9 @@ public:
>    void dispatch ();
>  
>  private:
> +  /* Prohibit copy construction.  */
> +  fma_forest (const fma_forest &);
> +
>    /* The list of roots that form this forest.  */
>    std::list<fma_root_node *> *m_roots;
>  
> @@ -147,6 +150,9 @@ public:
>    void set_head (du_head *);
>    void rename (fma_forest *);
>    void dump_info (fma_forest *);
> +private:

Blank line before 'private:'.

> +  /* Prohibit copy construction.  */
> +  fma_node (const fma_node &);
>  
>  protected:
>    /* Root node that lead to this node.  */
> @@ -203,6 +209,9 @@ public:
>    void execute_fma_steering ();
>  
>  private:
> +  /* Prohibit copy construction.  */
> +  func_fma_steering (const func_fma_steering &);
> +
>    void dfs (void (*) (fma_forest *), void (*) (fma_forest *, fma_root_node *),
>  	    void (*) (fma_forest *, fma_node *), bool);
>    void analyze ();
>
Andrea Corallo April 8, 2019, 2:18 p.m. UTC | #9
Richard Earnshaw (lists) writes:

> On 08/04/2019 09:59, Andrea Corallo wrote:
>>
>> Richard Earnshaw (lists) writes:
>>
>>> Ah, you've just changed the ChangeLog entries.  By comments, I meant in
>>> the source code, so that it's clear these don't exist.  ChangeLog update
>>> is good too.
>>>
>>> R.
>>
>> Hi Richard,
>> sorry my misunderstanding.
>>
>
> NP.  I've put this in.  For future reference, I've made a few very minor
> tweaks before applying...

Thanks for that!

Bests
  Andrea
diff mbox series

Patch

diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c b/gcc/config/aarch64/cortex-a57-fma-steering.c
index f2da03a..a390a62 100644
--- a/gcc/config/aarch64/cortex-a57-fma-steering.c
+++ b/gcc/config/aarch64/cortex-a57-fma-steering.c
@@ -114,6 +114,8 @@  public:
   void dispatch ();
 
 private:
+  fma_forest (const fma_forest &);
+
   /* The list of roots that form this forest.  */
   std::list<fma_root_node *> *m_roots;
 
@@ -180,6 +182,8 @@  public:
   void dump_info (fma_forest *);
 
 private:
+  fma_root_node (const fma_root_node &);
+
   /* The forest this node belonged to when it was created.  */
   fma_forest *m_forest;
 };
@@ -203,6 +207,7 @@  public:
   void execute_fma_steering ();
 
 private:
+  func_fma_steering (const func_fma_steering &);
   void dfs (void (*) (fma_forest *), void (*) (fma_forest *, fma_root_node *),
 	    void (*) (fma_forest *, fma_node *), bool);
   void analyze ();