Fix PR83033

Message ID gkr36n6t055.fsf@arm.com
State New
Headers show
Series
  • Fix PR83033
Related show

Commit Message

Andrea Corallo March 29, 2019, 11:01 a.m.
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. | #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. | #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. | #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. | #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. | #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. | #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. | #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 ();
-- 
2.7.4
Richard Earnshaw (lists) April 8, 2019, 2:04 p.m. | #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. | #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

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 ();