| Message ID | 1277231415.19557.20.camel@glinka |
|---|---|
| State | New |
| Headers | show |
On Tue, Jun 22, 2010 at 8:30 PM, Basile Starynkevitch <basile@starynkevitch.net> wrote: > Hello All, > > since gimple_seq_node is not very useful in coretypes.h I bootstrapped > the following attached patch to trunk rev.161214 on > x86_64-unknown-linux-gnu with c,c++,lto languages. > > gcc/ChangeLog entry: > > 2010-06-22 Basile Starynkevitch <basile@starynkevitch.net> > * coretypes.h (gimple_seq_node_d, gimple_seq_node) > (const_gimple_seq_node): Removed typedefs. > > * gimple.h (gimple_seq_node_d, gimple_seq_node) > (const_gimple_seq_node): Added typedefs moved from coretypes.h. Please do not do this. gimple_seq is used in target.h for example, but target.h should not have to include gimple.h. This patch is a step in the wrong direction. Ciao! Steven
On Tue, 2010-06-22 at 22:04 +0200, Steven Bosscher wrote: > On Tue, Jun 22, 2010 at 8:30 PM, Basile Starynkevitch > <basile@starynkevitch.net> wrote: > > Hello All, > > > > since gimple_seq_node is not very useful in coretypes.h I bootstrapped > > the following attached patch to trunk rev.161214 on > > x86_64-unknown-linux-gnu with c,c++,lto languages. > > > > gcc/ChangeLog entry: > > > > 2010-06-22 Basile Starynkevitch <basile@starynkevitch.net> > > * coretypes.h (gimple_seq_node_d, gimple_seq_node) > > (const_gimple_seq_node): Removed typedefs. > > > > * gimple.h (gimple_seq_node_d, gimple_seq_node) > > (const_gimple_seq_node): Added typedefs moved from coretypes.h. > > Please do not do this. gimple_seq is used in target.h for example, but > target.h should not have to include gimple.h. > > This patch is a step in the wrong direction. But the patch did not remove the gimple_seq typedef, only the gimple_seq_node one. So I don't understand why you believe it is a wrong step. Why would target.h need gimple_seq_node without including gimple.h? BTW, the proposed patch did bootstrap, so I suppose that some code did compile correctly and included target.h. [Most passes work on gimple_seq, not gimple_seq_node]. Cheers.
On Tue, Jun 22, 2010 at 10:07 PM, Basile Starynkevitch <basile@starynkevitch.net> wrote: > But the patch did not remove the gimple_seq typedef, only the > gimple_seq_node one. So I don't understand why you believe it is a wrong > step. Ignore me, I misunderstood your patch. It is a good step. Ciao! Steven
On Tue, 2010-06-22 at 22:31 +0200, Steven Bosscher wrote: > On Tue, Jun 22, 2010 at 10:07 PM, Basile Starynkevitch > <basile@starynkevitch.net> wrote: > > But the patch did not remove the gimple_seq typedef, only the > > gimple_seq_node one. So I don't understand why you believe it is a wrong > > step. > > Ignore me, I misunderstood your patch. It is a good step. Should I understand that as an Ok to apply it?
On 6/23/10, Basile Starynkevitch <basile@starynkevitch.net> wrote: > On Tue, 2010-06-22 at 22:31 +0200, Steven Bosscher wrote: >> On Tue, Jun 22, 2010 at 10:07 PM, Basile Starynkevitch >> <basile@starynkevitch.net> wrote: >> > But the patch did not remove the gimple_seq typedef, only the >> > gimple_seq_node one. So I don't understand why you believe it is a wrong >> > step. >> >> Ignore me, I misunderstood your patch. It is a good step. > > > Should I understand that as an Ok to apply it? > I would say yes, but I can't approve patches. Ciao! Steven
On Tue, Jun 22, 2010 at 8:30 PM, Basile Starynkevitch <basile@starynkevitch.net> wrote: > Hello All, > > since gimple_seq_node is not very useful in coretypes.h I bootstrapped > the following attached patch to trunk rev.161214 on > x86_64-unknown-linux-gnu with c,c++,lto languages. > > gcc/ChangeLog entry: Ok with +/* The types gimple & gimple_seq are defined in coretypes.h but + gimple_seq_node is not needed there. */ removed. Thanks, Richard. > 2010-06-22 Basile Starynkevitch <basile@starynkevitch.net> > * coretypes.h (gimple_seq_node_d, gimple_seq_node) > (const_gimple_seq_node): Removed typedefs. > > * gimple.h (gimple_seq_node_d, gimple_seq_node) > (const_gimple_seq_node): Added typedefs moved from coretypes.h. > > See also http://gcc.gnu.org/ml/gcc/2010-06/msg00670.html & > http://gcc.gnu.org/ml/gcc/2010-06/msg00657.html > > By the way, I would also happy if someone could also review the > GNU-friendly gengtype patch > http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02178.html > > Cheers > -- > Basile STARYNKEVITCH http://starynkevitch.net/Basile/ > email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359 > 8, rue de la Faiencerie, 92340 Bourg La Reine, France > *** opinions {are only mines, sont seulement les miennes} *** > >
Index: gcc/coretypes.h =================================================================== --- gcc/coretypes.h (revision 161214) +++ gcc/coretypes.h (working copy) @@ -68,9 +68,6 @@ struct cl_optimization; struct gimple_seq_d; typedef struct gimple_seq_d *gimple_seq; typedef const struct gimple_seq_d *const_gimple_seq; -struct gimple_seq_node_d; -typedef struct gimple_seq_node_d *gimple_seq_node; -typedef const struct gimple_seq_node_d *const_gimple_seq_node; /* Address space number for named address space support. */ typedef unsigned char addr_space_t; Index: gcc/gimple.h =================================================================== --- gcc/gimple.h (revision 161214) +++ gcc/gimple.h (working copy) @@ -33,6 +33,12 @@ along with GCC; see the file COPYING3. If not see #include "tree-ssa-operands.h" #include "tree-ssa-alias.h" +/* The types gimple & gimple_seq are defined in coretypes.h but + gimple_seq_node is not needed there. */ +struct gimple_seq_node_d; +typedef struct gimple_seq_node_d *gimple_seq_node; +typedef const struct gimple_seq_node_d *const_gimple_seq_node; + /* For each block, the PHI nodes that need to be rewritten are stored into these vectors. */ typedef VEC(gimple, heap) *gimple_vec;