Message ID | gkr36n6t055.fsf@arm.com |
---|---|
State | New |
Headers | show |
Series | Fix PR83033 | expand |
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. >
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. > >
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 (); >
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 ();
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. >
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. >> >
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 ();
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 (); >
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 --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 ();