Patchwork trunk patch: moving gimple_seq_node from coretypes.h to gimple.h

login
register
mail settings
Submitter Basile Starynkevitch
Date June 22, 2010, 6:30 p.m.
Message ID <1277231415.19557.20.camel@glinka>
Download mbox | patch
Permalink /patch/56562/
State New
Headers show

Comments

Basile Starynkevitch - June 22, 2010, 6:30 p.m.
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.

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
Steven Bosscher - June 22, 2010, 8:04 p.m.
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
Basile Starynkevitch - June 22, 2010, 8:07 p.m.
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.
Steven Bosscher - June 22, 2010, 8:31 p.m.
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
Basile Starynkevitch - June 23, 2010, 5:02 a.m.
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?
Steven Bosscher - June 23, 2010, 6:21 a.m.
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
Richard Guenther - June 23, 2010, 9:32 a.m.
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} ***
>
>

Patch

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;