Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
Hi,
On Thu, 27 Sep 2012, Lawrence Crowl wrote:
template typename T
struct D : BT
{
typedef typename BT::E E; // element_type
E getme (int index);
}
Inside that struct, lets say we have a field of type E. Do we name
it F or f?
IMHO only for types, not for any other decls.
Do you have an alternate suggestion, one that does not confuse
template parameters and dependent names?
Upper last character? Just kidding :) Too many detailed rules
for conventions are the death of them, use rules of thumbs,
my one would be somehow depends on template args - has upper
character in name, where somehow depends on includes is a.
Ah, but there is a problem. That typedef name does not necessarily
depend on a template parameter.
It is common practice to have
struct Q
{
typedef int E;
E getme (int index);
};
Easy: I wouldn't make a typedef for Q::E that merely is int. The reason
is that it makes knowing what getme really returns harder. You have to
look it up always (or know the class already). In fact that's one of my
gripes with the standard library, much too much indirection through
entities merely referring to other entities. Might be only important for
the libraries implementors but I sure hope that we don't start down that
road in GCC.
In fact, one place is in the hash table code we are discussing.
The hash descriptor type may not itself be a template. I believe
that few of them will actually be templates.
Then I don't see the need for class-local typedefs.
So, if E implies comes from template, the implication is wrong.
If we were to follow C++ standard library conventions, we would call it
value_type.
Well, but value_type surely does depend on the hashtables type argument,
doesn't it? After all it is a typedef from 'Key'.
I would expect that htabtree::value_type is tree, and
htabint::value_type is int, and I would like to see it named
htabtree::T or ::E.
That would be my preference. However, if folks want a shorter name,
I'll live with that too. But as it stands, the current name is very
confusing.
I would even prefer 'e' over value_type. It's scoped, the context always
will be clear, no need to be verbose in that name. I find the long names
inelegant, as most of the standard libs conventions.
Ciao,
Michael.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Fri, Sep 28, 2012 at 8:18 AM, Michael Matz m...@suse.de wrote:
It is common practice to have
struct Q
{
typedef int E;
E getme (int index);
};
Easy: I wouldn't make a typedef for Q::E that merely is int. The reason
is that it makes knowing what getme really returns harder.
The point of these nested type is precisely to allow a *uniform* access
to associated from within a template (e.g. a container) -- irrespective of
what those types happen to resolve to, builtin or not.
-- Gaby
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 9/28/12, Michael Matz m...@suse.de wrote:
On Thu, 27 Sep 2012, Lawrence Crowl wrote:
template typename T
struct D : BT
{
typedef typename BT::E E; // element_type
E getme (int index);
}
Inside that struct, lets say we have a field of type E. Do we name
it F or f?
IMHO only for types, not for any other decls.
Do you have an alternate suggestion, one that does not confuse
template parameters and dependent names?
Upper last character? Just kidding :) Too many detailed rules
for conventions are the death of them, use rules of thumbs,
my one would be somehow depends on template args - has upper
character in name, where somehow depends on includes is a.
Ah, but there is a problem. That typedef name does not necessarily
depend on a template parameter.
It is common practice to have
struct Q
{
typedef int E;
E getme (int index);
};
Easy: I wouldn't make a typedef for Q::E that merely is int. The reason
is that it makes knowing what getme really returns harder. You have to
look it up always (or know the class already). In fact that's one of my
gripes with the standard library, much too much indirection through
entities merely referring to other entities. Might be only important for
the libraries implementors but I sure hope that we don't start down that
road in GCC.
In fact, one place is in the hash table code we are discussing.
The hash descriptor type may not itself be a template. I believe
that few of them will actually be templates.
Then I don't see the need for class-local typedefs.
So, if E implies comes from template, the implication is wrong.
If we were to follow C++ standard library conventions, we would call it
value_type.
Well, but value_type surely does depend on the hashtables
type argument, doesn't it? After all it is a typedef from
'Key'. I would expect that htabtree::value_type is tree, and
htabint::value_type is int, and I would like to see it named
htabtree::T or ::E.
One declares a hash table as follows.
hash_table hash_descriptor variable;
The type stored in the hash table is not part of the declaration.
It is part of the descriptor, along with other things like the
hash function. The hash table essentially queries the descriptor
for the value type. For example,
template Descriptor class hash_table {
typename Descriptor::value_type *storage;
...
More typically though, the typedef is repeated inside the class to
avoid excess verbosity, and more importantly, to reexport the name.
template Descriptor class hash_table {
typedef typename Descriptor::value_type value_type;
value_type *storage;
...
Using these typedef names is an essential component of the
abstraction. Without it, we end up going to void* and loosing all
type safety.
That would be my preference. However, if folks want a shorter name,
I'll live with that too. But as it stands, the current name is very
confusing.
I would even prefer 'e' over value_type. It's scoped, the context always
will be clear, no need to be verbose in that name. I find the long names
inelegant, as most of the standard libs conventions.
We need some convention. If we choose a convention different from
the standard library, then we are essentially saying that we do not
intend to interoperate with the standard library. I do not think
that is the intent of the community, but I could be wrong about that.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Fri, Sep 28, 2012 at 12:40 PM, Lawrence Crowl cr...@google.com wrote: On 9/28/12, Michael Matz m...@suse.de wrote: I would even prefer 'e' over value_type. It's scoped, the context always will be clear, no need to be verbose in that name. I find the long names inelegant, as most of the standard libs conventions. We need some convention. If we choose a convention different from the standard library, then we are essentially saying that we do not intend to interoperate with the standard library. I do not think that is the intent of the community, but I could be wrong about that. I agree. If there already exists a convention that is widely known and recognized, then we should use it. There is negative value in inventing a new convention. We need to lower barriers to adoption, not raise them. Using the standard library convention seems to me like the best thing to do here. Diego.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 9/28/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Fri, Sep 28, 2012 at 8:18 AM, Michael Matz m...@suse.de wrote:
It is common practice to have
struct Q
{
typedef int E;
E getme (int index);
};
Easy: I wouldn't make a typedef for Q::E that merely is int. The reason
is that it makes knowing what getme really returns harder.
The point of these nested type is precisely to allow a *uniform* access
to associated from within a template (e.g. a container) -- irrespective of
what those types happen to resolve to, builtin or not.
Perhaps an analogy might be helpful. If I say Tim's father you know
role of that person without knowing exactly who it is. The typedef
convention serves the same purpose. It is important that we use the
same term for father, or we wouldn't be able to communicate.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
Hi,
On Wed, 26 Sep 2012, Lawrence Crowl wrote:
A lower-case type name indicates to me a non-changing type,
i.e. nothing that depends on a template. In C we only had
such types so we used lower-case names everywhere. With C++
and templates I think we should start using upper case for some
very specific use cases, like first letter of dependend types.
How would you distinguish them from template parameter names,
which by convention have an upper case first letter?
I wouldn't. If the distinction becomes so important that authors need to
see the speciality immediately by having a different convention how to
spell names, then I think we did something wrong, and we should simplify
the code.
What about non-type dependent names?
I'm not sure what you're asking. Let's make an example:
template typename T
struct D : BT
{
typedef typename BT::E E; // element_type
E getme (int index);
}
In fact, as BT::E would probably be defined like typedef typename T E,
I would even have no issue to call the above E also T. The distinction
between the template arg name and the typedef would be blurred, and I say,
so what; one is a typedef of the other and hence mostly equivalent for
practical purposes. (And if they aren't, then again, we did something too
complicated with the switch to C++).
The advantage to following them is that they will surprise no one.
They will surprise everyone used to different conventions, for instance
Qt, so that's not a reason.
Do you have an alternate suggestion, one that does not confuse template
parameters and dependent names?
Upper last character? Just kidding :) Too many detailed rules for
conventions are the death of them, use rules of thumbs, my one would be
somehow depends on template args - has upper character in name, where
somehow depends on includes is a.
Ciao,
Michael.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Thu, Sep 27, 2012 at 7:12 AM, Michael Matz m...@suse.de wrote: (And if they aren't, then again, we did something too complicated with the switch to C++). or we are doing something by insisting not to use standard notation.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 9/27/12, Michael Matz m...@suse.de wrote:
On Wed, 26 Sep 2012, Lawrence Crowl wrote:
A lower-case type name indicates to me a non-changing type,
i.e. nothing that depends on a template. In C we only had
such types so we used lower-case names everywhere. With C++
and templates I think we should start using upper case for some
very specific use cases, like first letter of dependend types.
How would you distinguish them from template parameter names,
which by convention have an upper case first letter?
I wouldn't. If the distinction becomes so important that authors
need to see the speciality immediately by having a different
convention how to spell names, then I think we did something wrong,
and we should simplify the code.
What about non-type dependent names?
I'm not sure what you're asking. Let's make an example:
template typename T
struct D : BT
{
typedef typename BT::E E; // element_type
E getme (int index);
}
Inside that struct, lets say we have a field of type E. Do we name
it F or f?
In fact, as BT::E would probably be defined like typedef
typename T E, I would even have no issue to call the above E
also T. The distinction between the template arg name and the
typedef would be blurred, and I say, so what; one is a typedef
of the other and hence mostly equivalent for practical purposes.
(And if they aren't, then again, we did something too complicated
with the switch to C++).
The advantage to following them is that they will surprise
no one.
They will surprise everyone used to different conventions, for
instance Qt, so that's not a reason.
Anyone using the standard library will not be surprised if we follow
the conventions of the standard library. I'd guess that the number
standard library programmers outnumbers the Qt programmers by 100
to 1. I'd guess that the number of Qt programmers that do not know
the standard library is a minority.
Do you have an alternate suggestion, one that does not confuse
template parameters and dependent names?
Upper last character? Just kidding :) Too many detailed rules
for conventions are the death of them, use rules of thumbs,
my one would be somehow depends on template args - has upper
character in name, where somehow depends on includes is a.
Ah, but there is a problem. That typedef name does not necessarily
depend on a template parameter.
It is common practice to have
struct Q
{
typedef int E;
E getme (int index);
};
and use it in exactly the same places you would use Dsomething.
In fact, one place is in the hash table code we are discussing.
The hash descriptor type may not itself be a template. I believe
that few of them will actually be templates.
So, if E implies comes from template, the implication is wrong.
If we were to follow C++ standard library conventions, we would call
it value_type. That would be my preference. However, if folks
want a shorter name, I'll live with that too. But as it stands,
the current name is very confusing.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Thu, Sep 27, 2012 at 1:35 PM, Lawrence Crowl cr...@google.com wrote: If we were to follow C++ standard library conventions, we would call it value_type. That would be my preference. However, if folks want a shorter name, I'll live with that too. But as it stands, the current name is very confusing. Yes, and there appears to be no good reason to let it stand. -- Gaby
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
Hi, On Tue, 25 Sep 2012, Lawrence Crowl wrote: On 8/15/12, Richard Henderson r...@redhat.com wrote: On 2012-08-15 07:29, Richard Guenther wrote: + typedef typename Element::Element_t Element_t; Can we use something less ugly than Element_t? Such as typedef typename Element::T T; ? Given that this name is scoped anyway... I've been finding the use of T as a typedef confusing. Why? As type placeholder in templates it's quite customary, as the most interesting to users of this template. I would have no trouble at all to see declarations like T x = getme(); in a function. In some way I even prefer that to some lower-case variant, because it reminds me that this specific T is actually variant. A lower-case type name indicates to me a non-changing type, i.e. nothing that depends on a template. In C we only had such types so we used lower-case names everywhere. With C++ and templates I think we should start using upper case for some very specific use cases, like first letter of dependend types. It sort of flies in the face of all existing convention. If you talk about the conventions used for the c++ standard library, then they are IMHO quite ugly and we should not follow them. Ciao, Michael.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Tue, Sep 25, 2012 at 4:30 PM, Lawrence Crowl cr...@google.com wrote: On 8/15/12, Richard Henderson r...@redhat.com wrote: On 2012-08-15 07:29, Richard Guenther wrote: + typedef typename Element::Element_t Element_t; Can we use something less ugly than Element_t? Such as typedef typename Element::T T; ? Given that this name is scoped anyway... I've been finding the use of T as a typedef confusing. It sort of flies in the face of all existing convention. The C++ standard would use either element_type or value_type. I suggest a rename, but I'm guessing that folks don't want something as verbose as element_type. How about elemtype? Any objections to me changing it to that? In general, we would be better off following standard convention. It does not seem to me that we have enough reasons and benefits in building our own universe around this issue. Consequently, I would suggest that we use value_type or element_type in the interface -- which is better served being readable: this is the *interface*. I am pretty sure that people will quickly resort to local typedefs that are more readable even if we adopt a short (therefore cryptic) abbreviations such as T in the interface. It is one more hurdle that has no reason to be. -- Gaby
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 9/26/12, Michael Matz m...@suse.de wrote: On Tue, 25 Sep 2012, Lawrence Crowl wrote: On 8/15/12, Richard Henderson r...@redhat.com wrote: On 2012-08-15 07:29, Richard Guenther wrote: typedef typename Element::Element_t Element_t; Can we use something less ugly than Element_t? Such as typedef typename Element::T T; ? Given that this name is scoped anyway... I've been finding the use of T as a typedef confusing. Why? As type placeholder in templates it's quite customary, as the most interesting to users of this template. I would have no trouble at all to see declarations like T x = getme(); in a function. In some way I even prefer that to some lower-case variant, because it reminds me that this specific T is actually variant. The problem is that while T is customary as a template parameter, I have never seen it used as a typedef name. And that's the situation that we are in now. A lower-case type name indicates to me a non-changing type, i.e. nothing that depends on a template. In C we only had such types so we used lower-case names everywhere. With C++ and templates I think we should start using upper case for some very specific use cases, like first letter of dependend types. How would you distinguish them from template parameter names, which by convention have an upper case first letter? What about non-type dependent names? I think we really do need a separate convention for the two, because dependent members of class templates often need special access syntax. It sort of flies in the face of all existing convention. If you talk about the conventions used for the c++ standard library, then they are IMHO quite ugly and we should not follow them. The advantage to following them is that they will surprise no one. Do you have an alternate suggestion, one that does not confuse template parameters and dependent names? -- Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Wed, Sep 26, 2012 at 1:09 PM, Lawrence Crowl cr...@google.com wrote: The problem is that while T is customary as a template parameter, I have never seen it used as a typedef name. And that's the situation that we are in now. this should be a no-brainer: T should be reserved for the name of the template parameter. [...] I think we really do need a separate convention for the two, because dependent members of class templates often need special access syntax. Yes. -- Gaby
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 8/15/12, Richard Henderson r...@redhat.com wrote: On 2012-08-15 07:29, Richard Guenther wrote: + typedef typename Element::Element_t Element_t; Can we use something less ugly than Element_t? Such as typedef typename Element::T T; ? Given that this name is scoped anyway... I've been finding the use of T as a typedef confusing. It sort of flies in the face of all existing convention. The C++ standard would use either element_type or value_type. I suggest a rename, but I'm guessing that folks don't want something as verbose as element_type. How about elemtype? Any objections to me changing it to that? -- Lawrence Crowl
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Thu, Aug 23, 2012 at 01:54:38PM -0700, Mike Stump wrote: I think: # Remove the -O2: for historical reasons, unless bootstrapping we prefer # optimizations to be activated explicitly by the toplevel. case $CC in */prev-gcc/xgcc*) ;; *) CFLAGS=`echo $CFLAGS | sed s/-O[[s0-9]]* *// ` ;; esac AC_SUBST(CFLAGS) in configure.ac does this. I think if CXXFLAGS is also so done, we'd gain parity. Can we get this change in? The current state is terribly annoying. Jakub
Re: Merge C++ conversion into trunk (0/6 - Overview)
Il 13/09/2012 10:46, Jakub Jelinek ha scritto: # Remove the -O2: for historical reasons, unless bootstrapping we prefer # optimizations to be activated explicitly by the toplevel. case $CC in */prev-gcc/xgcc*) ;; *) CFLAGS=`echo $CFLAGS | sed s/-O[[s0-9]]* *// ` ;; esac AC_SUBST(CFLAGS) in configure.ac does this. I think if CXXFLAGS is also so done, we'd gain parity. Can we get this change in? The current state is terribly annoying. Yes, please go ahead. Paolo
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Thu, Sep 13, 2012 at 10:53:23AM +0200, Paolo Bonzini wrote: Il 13/09/2012 10:46, Jakub Jelinek ha scritto: # Remove the -O2: for historical reasons, unless bootstrapping we prefer # optimizations to be activated explicitly by the toplevel. case $CC in */prev-gcc/xgcc*) ;; *) CFLAGS=`echo $CFLAGS | sed s/-O[[s0-9]]* *// ` ;; esac AC_SUBST(CFLAGS) in configure.ac does this. I think if CXXFLAGS is also so done, we'd gain parity. Can we get this change in? The current state is terribly annoying. Yes, please go ahead. Here it is, bootstrapped/regtested on x86_64-linux and i686-linux, additionally tested on --disable-bootstrap tree, both by make cc1 inside of gcc subdir (no -O2) and make all-gcc above it (with -O2). Ok for trunk? 2012-09-13 Jakub Jelinek ja...@redhat.com * configure.ac (CXXFLAGS): Remove -O2 when not bootstrapping. * configure: Regenerated. --- gcc/configure.ac.jj 2012-09-13 07:54:41.0 +0200 +++ gcc/configure.ac2012-09-13 14:19:54.016741197 +0200 @@ -296,9 +296,11 @@ AC_SUBST(OUTPUT_OPTION) # optimizations to be activated explicitly by the toplevel. case $CC in */prev-gcc/xgcc*) ;; - *) CFLAGS=`echo $CFLAGS | sed s/-O[[s0-9]]* *// ` ;; + *) CFLAGS=`echo $CFLAGS | sed s/-O[[s0-9]]* *// ` + CXXFLAGS=`echo $CXXFLAGS | sed s/-O[[s0-9]]* *// ` ;; esac AC_SUBST(CFLAGS) +AC_SUBST(CXXFLAGS) # Determine PICFLAG for target gnatlib. GCC_PICFLAG_FOR_TARGET --- gcc/configure.jj2012-09-13 07:54:39.0 +0200 +++ gcc/configure 2012-09-13 14:34:40.429269215 +0200 @@ -4863,10 +4863,12 @@ fi # optimizations to be activated explicitly by the toplevel. case $CC in */prev-gcc/xgcc*) ;; - *) CFLAGS=`echo $CFLAGS | sed s/-O[s0-9]* *// ` ;; + *) CFLAGS=`echo $CFLAGS | sed s/-O[s0-9]* *// ` + CXXFLAGS=`echo $CXXFLAGS | sed s/-O[s0-9]* *// ` ;; esac + # Determine PICFLAG for target gnatlib. @@ -17782,7 +17784,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 17785 configure +#line 17787 configure #include confdefs.h #if HAVE_DLFCN_H @@ -17888,7 +17890,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 17891 configure +#line 17893 configure #include confdefs.h #if HAVE_DLFCN_H Jakub
Re: Merge C++ conversion into trunk (0/6 - Overview)
Il 13/09/2012 17:57, Jakub Jelinek ha scritto: Can we get this change in? The current state is terribly annoying. Yes, please go ahead. Here it is, bootstrapped/regtested on x86_64-linux and i686-linux, additionally tested on --disable-bootstrap tree, both by make cc1 inside of gcc subdir (no -O2) and make all-gcc above it (with -O2). Ok. Paolo
Re: Merge C++ conversion into trunk (6/6 - gdb tree macro support)
Diego Novillo dnovi...@google.com writes: +# Skip all inline functions in tree.h. +# These are used in accessor macros. +# Note that this is added at the end because older gdb versions +# do not understand the 'skip' command. +skip tree.h (gdb) skip tree.h No function found named tree.h. Committed as obvious. Andreas. * gdbinit.in: Fix syntax of skip command. diff --git a/gcc/gdbinit.in b/gcc/gdbinit.in index 79ffc09..0ddb91b 100644 --- a/gcc/gdbinit.in +++ b/gcc/gdbinit.in @@ -1,5 +1,5 @@ # Copyright (C) 2001, 2002, 2003, 2004, 2006, -# 2008, 2010 Free Software Foundation, Inc. +# 2008, 2010, 2012 Free Software Foundation, Inc. # # This file is part of GCC. # @@ -208,4 +208,4 @@ b abort # These are used in accessor macros. # Note that this is added at the end because older gdb versions # do not understand the 'skip' command. -skip tree.h +skip file tree.h -- 1.7.12 -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
Re: Merge C++ conversion into trunk (0/6 - Overview)
Il 23/08/2012 22:54, Mike Stump ha scritto: # Remove the -O2: for historical reasons, unless bootstrapping we prefer # optimizations to be activated explicitly by the toplevel. case $CC in */prev-gcc/xgcc*) ;; *) CFLAGS=`echo $CFLAGS | sed s/-O[[s0-9]]* *// ` ;; esac AC_SUBST(CFLAGS) in configure.ac does this. I think if CXXFLAGS is also so done, we'd gain parity. Agreed, patch is preapproved. This is not really done to aid debugging though, it is to avoid optimization bugs when compiling stage1. Paolo
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Aug 24, 2012, at 12:24 AM, Paolo Bonzini wrote: Agreed, patch is preapproved. This is not really done to aid debugging though, it is to avoid optimization bugs when compiling stage1. Ah, but building a non-bootstrap compiler from the top-level builds -O2 and when built from the gcc subtree, builds -O0. Ever wonder why? It isn't to avoid code-gen errors in CC. it is to make the developers life easier. I know, so much of gcc's history is lost to time and at times, not handed down to the new kids. The bad bits, just fade away. The useful things, for example, this, will live on, as some of use still know about and use the feature. When it breaks, we complain.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 2012-08-23 16:54 , Mike Stump wrote: On Aug 12, 2012, at 1:04 PM, Diego Novillo wrote: Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. In my gcc/Makefile, I see: CFLAGS = -g CXXFLAGS = -g -O2 Odd. I see: CFLAGS = -g CXXFLAGS = -g in stage1-gcc/ In gcc/ I see both set to -g -O2, of course. I would've noticed -g -O2 in the stage1 build because I am constantly debugging that binary. Diego.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Fri, Aug 24, 2012 at 08:30:36AM -0400, Diego Novillo wrote: On 2012-08-23 16:54 , Mike Stump wrote: On Aug 12, 2012, at 1:04 PM, Diego Novillo wrote: Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. In my gcc/Makefile, I see: CFLAGS = -g CXXFLAGS = -g -O2 Odd. I see: CFLAGS = -g CXXFLAGS = -g in stage1-gcc/ In gcc/ I see both set to -g -O2, of course. I would've noticed -g -O2 in the stage1 build because I am constantly debugging that binary. You haven't built your compiler with --disable-bootstrap, so you aren't seeing what Mike is complaining about. Jakub
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 2012-08-24 08:35 , Jakub Jelinek wrote: You haven't built your compiler with --disable-bootstrap, so you aren't seeing what Mike is complaining about. Ah, Mike failed to mention that detail. Mike, it is unlikely that I will be able to work on a fix before I leave. It does not look like a difficult fix in any case. Would you mind preparing a patch? If not, I'll get to it after I return. Thanks. Diego.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Aug 24, 2012, at 5:35 AM, Jakub Jelinek wrote: You haven't built your compiler with --disable-bootstrap, so you aren't seeing what Mike is complaining about. Actually, I'm not using disable Just a normal cross compile.
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Aug 12, 2012, at 1:15 PM, Diego Novillo wrote: + Second, the GCC conding conventions prefer explicit conversion, Spelling... coding
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Aug 12, 2012, at 1:04 PM, Diego Novillo wrote: Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. In my gcc/Makefile, I see: CFLAGS = -g CXXFLAGS = -g -O2 This makes builds in the gcc directory default to -O2, before they were -O0. -O0 aides in debugging for people that develop the compiler. Do you see this? I think: # Remove the -O2: for historical reasons, unless bootstrapping we prefer # optimizations to be activated explicitly by the toplevel. case $CC in */prev-gcc/xgcc*) ;; *) CFLAGS=`echo $CFLAGS | sed s/-O[[s0-9]]* *// ` ;; esac AC_SUBST(CFLAGS) in configure.ac does this. I think if CXXFLAGS is also so done, we'd gain parity.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Tue, Aug 21, 2012 at 3:31 AM, Lawrence Crowl cr...@google.com wrote: On 8/20/12, H.J. Lu hjl.to...@gmail.com wrote: The C++ merge caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54332 GCC memory usage is more than doubled from = 3GB to = 10GB. Is this a known issue? The two memory stat reports show no differences. Are you sure you didn't splice in the wrong report? Well, easy things such as a messed up hashtab conversion (no freeing) or vec conversion (no freeing) can cause this, or even the gengtype change causing GC issues (which is why those should have been different revisions). Richard. -- Lawrence Crowl
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Tue, Aug 14, 2012 at 11:59 AM, Diego Novillo dnovi...@google.com wrote: On 12-08-14 09:48 , Diego Novillo wrote: This merge touches several files, so I'm thinking that the best time is going to be on Thu 16/Aug around 2:00 GMT. So, the fixes I needed from Lawrence are already in so we can proceed with the merge. I'll commit the merge tonight at ~2:00 GMT. After the merge is in, I will send an announcement and request major branch merges to wait for another 24 hrs to allow testers the chance to pick up this merge. The C++ merge caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54332 GCC memory usage is more than doubled from = 3GB to = 10GB. Is this a known issue? -- H.J.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 8/20/12, H.J. Lu hjl.to...@gmail.com wrote: The C++ merge caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54332 GCC memory usage is more than doubled from = 3GB to = 10GB. Is this a known issue? The two memory stat reports show no differences. Are you sure you didn't splice in the wrong report? -- Lawrence Crowl
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Mon, Aug 20, 2012 at 6:31 PM, Lawrence Crowl cr...@google.com wrote: On 8/20/12, H.J. Lu hjl.to...@gmail.com wrote: The C++ merge caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54332 GCC memory usage is more than doubled from = 3GB to = 10GB. Is this a known issue? The two memory stat reports show no differences. Are you sure you didn't splice in the wrong report? Those are outputs from -fmem-report. I am not sure how useful they are for this bug. -- H.J.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 08/15/2012 11:25 AM, Gabriel Dos Reis wrote: On Wed, Aug 15, 2012 at 1:21 PM, Tom Tromey tro...@redhat.com wrote: Gaby == Gabriel Dos Reis g...@integrable-solutions.net writes: Tom I asked Keith to resurrect his patch for this. [snip] I hope this will be acceptable all around. OK, that sounds reasonable. I have committed a patch which should allow you to do this and closed c++/13356. If there are any further problems/issues, please let me know. We've re-purposed an old, unused option, check type. This option does absolutely nothing in older versions of gdb, so setting it unconditionally is safe for all recent versions of gdb: set check type off. Keith
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Fri, Aug 17, 2012 at 10:45 AM, Keith Seitz kei...@redhat.com wrote: On 08/15/2012 11:25 AM, Gabriel Dos Reis wrote: On Wed, Aug 15, 2012 at 1:21 PM, Tom Tromey tro...@redhat.com wrote: Gaby == Gabriel Dos Reis g...@integrable-solutions.net writes: Tom I asked Keith to resurrect his patch for this. [snip] I hope this will be acceptable all around. OK, that sounds reasonable. I have committed a patch which should allow you to do this and closed c++/13356. If there are any further problems/issues, please let me know. We've re-purposed an old, unused option, check type. This option does absolutely nothing in older versions of gdb, so setting it unconditionally is safe for all recent versions of gdb: set check type off. Shouldn't it be added to GCC .gdbinit? -- H.J.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Wed, 15 Aug 2012, Lawrence Crowl wrote:
On 8/15/12, Richard Henderson r...@redhat.com wrote:
On 2012-08-15 07:29, Richard Guenther wrote:
+ typedef typename Element::Element_t Element_t;
Can we use something less ugly than Element_t?
Such as
typedef typename Element::T T;
? Given that this name is scoped anyway...
I do not much like _t names either.
The following is what I'm testing now, it also integrates the
hashtable support functions and typedef within the existing local
data types which is IMHO cleaner. (it also shows we can do with
a janitorial cleanup replacing typedef struct foo_d {} foo; with
struct foo {}; and the likes)
Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu, ok?
Thanks,
Richard.
2012-08-16 Richard Guenther rguent...@suse.de
* hash-table.h (class hash_table): Use a descriptor template
argument instead of decomposed element type and support
functions.
(struct pointer_hash): New generic typed pointer-hash.
(struct typed_free_remove, struct typed_noop_remove): Generic
hash_table support pieces.
* coverage.c (struct counts_entry): Add hash_table support
members.
* tree-ssa-ccp.c (gimple_htab): Use pointer_hash.
* tree-ssa-coalesce.c (struct ssa_name_var_hash): New generic
SSA name by SSA_NAME_VAR hash.
(coalesce_ssa_name): Use it.
* tree-ssa-pre.c (struct pre_expr_d): Add hash_table support.
(expression_to_id): Adjust.
(struct expr_pred_trans_d): Add hash_table support.
(phi_translate_table): Adjust.
(phi_trans_lookup): Likewise.
(phi_trans_add): Likewise.
(do_regular_insertion): Likewise.
* tree-ssa-tail-merge.c (struct same_succ_def): Add hash_table
support.
(same_succ_htab): Adjust.
(find_same_succ_bb): Likewise.
(find_same_succ): Likewise.
(update_worklist): Likewise.
* tree-ssa-threadupdate.c (struct redirection_data): Add hash_table
support.
(redirection_data): Adjust.
Index: gcc/hash-table.h
===
*** gcc/hash-table.h.orig 2012-08-16 10:33:59.0 +0200
--- gcc/hash-table.h2012-08-16 11:08:36.311277498 +0200
*** xcallocator Type::data_free (Type *mem
*** 83,127
}
! /* A common function for hashing a CANDIDATE typed pointer. */
template typename Element
! inline hashval_t
! typed_pointer_hash (const Element *candidate)
{
! /* This is a really poor hash function, but it is what the current code
uses,
! so I am reusing it to avoid an additional axis in testing. */
! return (hashval_t) ((intptr_t)candidate 3);
! }
!
! /* A common function for comparing an EXISTING and CANDIDATE typed pointers
!for equality. */
template typename Element
! inline int
! typed_pointer_equal (const Element *existing, const Element * candidate)
{
! return existing == candidate;
! }
! /* A common function for doing nothing on removing a RETIRED slot. */
template typename Element
! inline void
! typed_null_remove (Element *retired ATTRIBUTE_UNUSED)
{
! }
! /* A common function for using free on removing a RETIRED slot. */
template typename Element
! inline void
! typed_free_remove (Element *retired)
{
! free (retired);
}
--- 83,134
}
! /* Remove method dispatching to free. */
template typename Element
! struct typed_free_remove
{
! static inline void remove (Element *p) { free (p); }
! };
! /* No-op remove method. */
template typename Element
! struct typed_noop_remove
{
! static inline void remove (Element *) {}
! };
! /* Pointer hash with a no-op remove method. */
template typename Element
! struct pointer_hash : typed_noop_remove Element
{
! typedef Element T;
+ static inline hashval_t
+ hash (const T *);
! static inline int
! equal (const T *existing, const T * candidate);
! };
template typename Element
! inline hashval_t
! pointer_hashElement::hash (const T *candidate)
{
! /* This is a really poor hash function, but it is what the current code
uses,
! so I am reusing it to avoid an additional axis in testing. */
! return (hashval_t) ((intptr_t)candidate 3);
! }
!
! template typename Element
! inline int
! pointer_hashElement::equal (const T *existing,
! const T *candidate)
! {
! return existing == candidate;
}
*** extern hashval_t hash_table_mod2 (hashva
*** 147,157
/* Internal implementation type. */
! template typename Element
struct hash_table_control
{
/* Table itself. */
! Element **entries;
/* Current size (in entries) of the hash table. */
size_t size;
--- 154,164
/* Internal implementation type. */
! template typename T
struct hash_table_control
{
/* Table itself.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
Hi,
I have another out of curiosity-type question ;)
On 08/16/2012 11:19 AM, Richard Guenther wrote:
!
! template typename Element
! inline int
! pointer_hashElement::equal (const T *existing,
! const T *candidate)
! {
! return existing == candidate;
}
are these uses in the new code of int instead of bool intended or
historical? Seem weird, definitely from the C++ (but even from the C)
point of view.
Paolo.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Thu, 16 Aug 2012, Paolo Carlini wrote:
Hi,
I have another out of curiosity-type question ;)
On 08/16/2012 11:19 AM, Richard Guenther wrote:
!
! template typename Element
! inline int
! pointer_hashElement::equal (const T *existing,
! const T *candidate)
! {
! return existing == candidate;
}
are these uses in the new code of int instead of bool intended or
historical? Seem weird, definitely from the C++ (but even from the C) point
of view.
Historical, copying what libiberty htab did.
Richard.
Re: Merge C++ conversion into trunk
On 12-08-16 12:50 , Iain Sandoe wrote: On 14 Aug 2012, at 19:59, Diego Novillo wrote: After the merge is in, I will send an announcement and request major branch merges to wait for another 24 hrs to allow testers the chance to pick up this merge. The following patch (mimicking what has been done elsewhere at r190402) restores bootstrap for powerpc-darwin. OK for trunk? Iain gcc/ * config/rs6000/rs6000.c (macho_branch_islands): Adjust for changes to vec.h. OK, thanks. Diego.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 8/16/12, Richard Guenther rguent...@suse.de wrote:
On Wed, 15 Aug 2012, Lawrence Crowl wrote:
On 8/15/12, Richard Henderson r...@redhat.com wrote:
On 2012-08-15 07:29, Richard Guenther wrote:
+ typedef typename Element::Element_t Element_t;
Can we use something less ugly than Element_t?
Such as
typedef typename Element::T T;
? Given that this name is scoped anyway...
I do not much like _t names either.
The following is what I'm testing now, it also integrates the
hashtable support functions and typedef within the existing local
data types which is IMHO cleaner. (it also shows we can do with
a janitorial cleanup replacing typedef struct foo_d {} foo; with
struct foo {}; and the likes)
Yes.
Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu, ok?
Looks good to me.
I would have prefered the Element-T rename in a separate patch
so that it is easier to see the core difference.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Tue, Aug 14, 2012 at 10:04 AM, Richard Guenther rguent...@suse.de wrote: On Mon, 13 Aug 2012, Lawrence Crowl wrote: On 8/13/12, Richard Guenther richard.guent...@gmail.com wrote: Increment/decrement operations did not exist, please do not add them at this point. Note that I have also added +=, -= and *= operations. Having them has three advantages. First, it matches expectations on what numeric types allow. Second, it results in more concise code. Third, it results in potentially faster code. I think we should be able to use those operators. When I run through changing call sites, I really want to change the sites to the final form, not do two passes. Ok. Btw, I noticed we now have /* Conversion functions. */ HOST_WIDE_INT to_signed () const; unsigned HOST_WIDE_INT to_unsigned () const; /* Conversion query functions. */ bool fits_unsigned () const; bool fits_signed () const; bool fits (bool uns) const; the function names make no sense - they should be talking about host-wide-ints, because that is what they are about. Thus, /* Conversion functions. */ HOST_WIDE_INT to_signed_hwi () const; unsigned HOST_WIDE_INT to_unsigned_hwi () const; /* Conversion query functions. */ bool fits_unsigned_hwi () const; bool fits_signed_hwi () const; bool fits_hwi (bool uns) const; Likewise for static double_int from_unsigned (unsigned HOST_WIDE_INT cst); static double_int from_signed (HOST_WIDE_INT cst); I'm going to install a patch, after testing, that adjusts the names accordingly. Richard. Thanks, Richard.
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Wed, Aug 15, 2012 at 12:28:58PM +0200, Richard Guenther wrote: the function names make no sense - they should be talking about host-wide-ints, because that is what they are about. Thus, /* Conversion functions. */ HOST_WIDE_INT to_signed_hwi () const; unsigned HOST_WIDE_INT to_unsigned_hwi () const; /* Conversion query functions. */ bool fits_unsigned_hwi () const; bool fits_signed_hwi () const; bool fits_hwi (bool uns) const; Wouldn't uhwi and shwi be better? Both are already widely used within gcc... Likewise for static double_int from_unsigned (unsigned HOST_WIDE_INT cst); static double_int from_signed (HOST_WIDE_INT cst); I'm going to install a patch, after testing, that adjusts the names accordingly. Jakub
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Wed, Aug 15, 2012 at 12:31 PM, Jakub Jelinek ja...@redhat.com wrote: On Wed, Aug 15, 2012 at 12:28:58PM +0200, Richard Guenther wrote: the function names make no sense - they should be talking about host-wide-ints, because that is what they are about. Thus, /* Conversion functions. */ HOST_WIDE_INT to_signed_hwi () const; unsigned HOST_WIDE_INT to_unsigned_hwi () const; /* Conversion query functions. */ bool fits_unsigned_hwi () const; bool fits_signed_hwi () const; bool fits_hwi (bool uns) const; Wouldn't uhwi and shwi be better? Both are already widely used within gcc... True, I'll change it that way. Richard. Likewise for static double_int from_unsigned (unsigned HOST_WIDE_INT cst); static double_int from_signed (HOST_WIDE_INT cst); I'm going to install a patch, after testing, that adjusts the names accordingly. Jakub
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Wed, 15 Aug 2012, Richard Guenther wrote:
On Wed, Aug 15, 2012 at 12:31 PM, Jakub Jelinek ja...@redhat.com wrote:
On Wed, Aug 15, 2012 at 12:28:58PM +0200, Richard Guenther wrote:
the function names make no sense - they should be talking about
host-wide-ints, because that is what they are about. Thus,
/* Conversion functions. */
HOST_WIDE_INT to_signed_hwi () const;
unsigned HOST_WIDE_INT to_unsigned_hwi () const;
/* Conversion query functions. */
bool fits_unsigned_hwi () const;
bool fits_signed_hwi () const;
bool fits_hwi (bool uns) const;
Wouldn't uhwi and shwi be better? Both are already widely used within
gcc...
True, I'll change it that way.
Like so, testing in progress.
Richard.
2012-08-15 Richard Guenther rguent...@suse.de
* double-int.h (from_unsigned): Rename to ...
(from_uhwi): ... this.
(from_signed): Rename to ...
(from_shwi): ... this.
(to_signed): Rename to ...
(to_shwi): ... this.
(to_unsigned): Rename to ...
(to_uhwi): ... this.
(fits_unsigned): Rename to ...
(fits_uhwi): ... this.
(fits_signed): Rename to ...
(fits_shwi): ... this.
(fits): Rename to ...
(fits_hwi): ... this.
* double-int.c: Likewise.
Index: gcc/double-int.c
===
*** gcc/double-int.c(revision 190407)
--- gcc/double-int.c(working copy)
*** double_int::sext (unsigned prec) const
*** 716,722
/* Returns true if CST fits in signed HOST_WIDE_INT. */
bool
! double_int::fits_signed () const
{
const double_int cst = *this;
if (cst.high == 0)
--- 716,722
/* Returns true if CST fits in signed HOST_WIDE_INT. */
bool
! double_int::fits_shwi () const
{
const double_int cst = *this;
if (cst.high == 0)
*** double_int::fits_signed () const
*** 731,742
unsigned HOST_WIDE_INT if UNS is true. */
bool
! double_int::fits (bool uns) const
{
if (uns)
! return this-fits_unsigned ();
else
! return this-fits_signed ();
}
/* Returns A * B. */
--- 731,742
unsigned HOST_WIDE_INT if UNS is true. */
bool
! double_int::fits_hwi (bool uns) const
{
if (uns)
! return this-fits_uhwi ();
else
! return this-fits_shwi ();
}
/* Returns A * B. */
Index: gcc/double-int.h
===
*** gcc/double-int.h(revision 190407)
--- gcc/double-int.h(working copy)
*** public:
*** 60,67
Second, the GCC conding conventions prefer explicit conversion,
and explicit conversion operators are not available until C++11. */
! static double_int from_unsigned (unsigned HOST_WIDE_INT cst);
! static double_int from_signed (HOST_WIDE_INT cst);
/* No copy assignment operator or destructor to keep the type a POD. */
--- 60,67
Second, the GCC conding conventions prefer explicit conversion,
and explicit conversion operators are not available until C++11. */
! static double_int from_uhwi (unsigned HOST_WIDE_INT cst);
! static double_int from_shwi (HOST_WIDE_INT cst);
/* No copy assignment operator or destructor to keep the type a POD. */
*** public:
*** 83,96
/* Conversion functions. */
! HOST_WIDE_INT to_signed () const;
! unsigned HOST_WIDE_INT to_unsigned () const;
/* Conversion query functions. */
! bool fits_unsigned () const;
! bool fits_signed () const;
! bool fits (bool uns) const;
/* Attribute query functions. */
--- 83,96
/* Conversion functions. */
! HOST_WIDE_INT to_shwi () const;
! unsigned HOST_WIDE_INT to_uhwi () const;
/* Conversion query functions. */
! bool fits_uhwi () const;
! bool fits_shwi () const;
! bool fits_hwi (bool uns) const;
/* Attribute query functions. */
*** public:
*** 186,192
HOST_WIDE_INT are filled with the sign bit. */
inline
! double_int double_int::from_signed (HOST_WIDE_INT cst)
{
double_int r;
r.low = (unsigned HOST_WIDE_INT) cst;
--- 186,192
HOST_WIDE_INT are filled with the sign bit. */
inline
! double_int double_int::from_shwi (HOST_WIDE_INT cst)
{
double_int r;
r.low = (unsigned HOST_WIDE_INT) cst;
*** double_int double_int::from_signed (HOST
*** 198,204
static inline double_int
shwi_to_double_int (HOST_WIDE_INT cst)
{
! return double_int::from_signed (cst);
}
/* Some useful constants. */
--- 198,204
static inline double_int
shwi_to_double_int (HOST_WIDE_INT cst)
{
! return double_int::from_shwi (cst);
}
/* Some useful constants. */
*** shwi_to_double_int (HOST_WIDE_INT cst)
*** 206,222
The problem is that a named constant would not be as optimizable,
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 12-08-15 07:59 , Richard Guenther wrote: (gdb) call debug_tree (*expr_p) integer_cst 0x7695d7c0 type integer_type 0x767fa5e8 int constant 9 (gdb) call debug_tree (0x767fa5e8) Cannot resolve function debug_tree to any overloaded instance Yeah, in the absence of overloads this is annoying. Happens with 0, too. Diego.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Wed, 15 Aug 2012, Diego Novillo wrote: On 12-08-15 07:59 , Richard Guenther wrote: (gdb) call debug_tree (*expr_p) integer_cst 0x7695d7c0 type integer_type 0x767fa5e8 int constant 9 (gdb) call debug_tree (0x767fa5e8) Cannot resolve function debug_tree to any overloaded instance Yeah, in the absence of overloads this is annoying. Happens with 0, too. 0 is fixed if you have recent enough gdb. (gdb) call debug_tree (0) as 0 is a null pointer constant. Richard.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 12-08-15 08:18 , Richard Guenther wrote: 0 is fixed if you have recent enough gdb. (gdb) call debug_tree (0) as 0 is a null pointer constant. Oh, cool. Progress. GDB folks, would it be hard to figure out that there is a single variant of the called function and trust the user that they are passing the right pointer value? Diego.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Wed, 15 Aug 2012, Richard Guenther wrote:
On Sun, 12 Aug 2012, Diego Novillo wrote:
This implements a new C++ hash table.
See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for
details.
Diego.
Now as we see the result I'd have prefered a more C++-way instead
of making the conversion simple ...
Like
template typename Element
class hash_table
{
...
};
template typename Element
class pointer_hash
{
hashval_t hash ();
int equal (const Element *);
~Element ();
Element e;
};
and
/* Hash table with all same_succ entries. */
static hash_table pointer_hash struct same_succ_def same_succ_htab;
The existing way is simply too ugly ... so, why did you not invent
a nice C++ way?
Like the following, only the coverage.c use is converted. I've
never seen template function arguments anywhere and having to repeat
them all over the place is really really ugly (yes, even if only in
the implementation).
This goes with static member functions and not wrapping any data.
It goes away with the requirement of having externally visible
global functions for the hash/compare functions as well (which was
ugly anyway).
Comments?
Thanks,
Richard.
diffstat ~/p
coverage.c | 43 +-
hash-table.h | 234
+--
2 files changed, 108 insertions(+), 169 deletions(-)
Index: gcc/hash-table.h
===
--- gcc/hash-table.h(revision 190410)
+++ gcc/hash-table.h(working copy)
@@ -199,45 +199,42 @@ struct hash_table_control
*/
template typename Element,
- hashval_t (*Hash) (const Element *candidate),
- int (*Equal) (const Element *existing, const Element * candidate),
- void (*Remove) (Element *retired),
template typename Type class Allocator = xcallocator
class hash_table
{
+public:
+ typedef typename Element::Element_t Element_t;
private:
+ hash_table_control Element_t *htab;
- hash_table_control Element *htab;
-
- Element **find_empty_slot_for_expand (hashval_t hash);
+ Element_t **find_empty_slot_for_expand (hashval_t hash);
void expand ();
public:
-
hash_table ();
void create (size_t initial_slots);
bool is_created ();
void dispose ();
- Element *find (Element *comparable);
- Element *find_with_hash (Element *comparable, hashval_t hash);
- Element **find_slot (Element *comparable, enum insert_option insert);
- Element **find_slot_with_hash (Element *comparable, hashval_t hash,
-enum insert_option insert);
+ Element_t *find (Element_t *comparable);
+ Element_t *find_with_hash (Element_t *comparable, hashval_t hash);
+ Element_t **find_slot (Element_t *comparable, enum insert_option insert);
+ Element_t **find_slot_with_hash (Element_t *comparable, hashval_t hash,
+ enum insert_option insert);
void empty ();
- void clear_slot (Element **slot);
- void remove_elt (Element *comparable);
- void remove_elt_with_hash (Element *comparable, hashval_t hash);
+ void clear_slot (Element_t **slot);
+ void remove_elt (Element_t *comparable);
+ void remove_elt_with_hash (Element_t *comparable, hashval_t hash);
size_t size();
size_t elements();
double collisions();
template typename Argument,
- int (*Callback) (Element **slot, Argument argument)
+ int (*Callback) (Element_t **slot, Argument argument)
void traverse_noresize (Argument argument);
template typename Argument,
- int (*Callback) (Element **slot, Argument argument)
+ int (*Callback) (Element_t **slot, Argument argument)
void traverse (Argument argument);
};
@@ -245,12 +242,9 @@ public:
/* Construct the hash table. The only useful operation next is create. */
template typename Element,
- hashval_t (*Hash) (const Element *candidate),
- int (*Equal) (const Element *existing, const Element * candidate),
- void (*Remove) (Element *retired),
template typename Type class Allocator
inline
-hash_table Element, Hash, Equal, Remove, Allocator::hash_table ()
+hash_table Element, Allocator::hash_table ()
: htab (NULL)
{
}
@@ -259,12 +253,9 @@ hash_table Element, Hash, Equal, Remove
/* See if the table has been created, as opposed to constructed. */
template typename Element,
- hashval_t (*Hash) (const Element *candidate),
- int (*Equal) (const Element *existing, const Element * candidate),
- void (*Remove) (Element *retired),
template typename Type class Allocator
inline bool
-hash_table Element, Hash, Equal, Remove, Allocator::is_created ()
+hash_table Element, Allocator::is_created ()
{
return htab != NULL;
}
@@ -273,56 +264,44 @@ hash_table Element, Hash, Equal, Remove
/* Like find_with_hash, but compute the hash value from the element. */
template typename Element,
- hashval_t (*Hash)
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
Hi, On Wed, 15 Aug 2012, Richard Guenther wrote: Like the following, only the coverage.c use is converted. I've never seen template function arguments anywhere and having to repeat them all over the place is really really ugly (yes, even if only in the implementation). This goes with static member functions and not wrapping any data. It goes away with the requirement of having externally visible global functions for the hash/compare functions as well (which was ugly anyway). Comments? Well, it looks nicer than what's there currently. As the element functions now are scoped and normal member functions, they should be named with lower case characters of course. I do like that the hash table would then only have one real template argument; the Allocator, well, no better idea comes to my mind. Ciao, Michael.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Aug 15, 2012, at 4:59 AM, Richard Guenther wrote: and debugging becomes a nightmare (hello gdb people!) (gdb) call debug_tree (0x767fa5e8) Cannot resolve function debug_tree to any overloaded instance Inquiring minds want to know if: macro define debug_tree(A) ((tree)A) makes the above work if you put it into gdbinit.in? If yes, then, I think for now we should add such lines for every non-overloaded function, I too like using hex constants from dumps. Oh, or, maybe we just add a debug_tree(long) overload just to satisfy gdb.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Wed, 15 Aug 2012, Michael Matz wrote:
Hi,
On Wed, 15 Aug 2012, Richard Guenther wrote:
Like the following, only the coverage.c use is converted. I've never
seen template function arguments anywhere and having to repeat them all
over the place is really really ugly (yes, even if only in the
implementation).
This goes with static member functions and not wrapping any data. It
goes away with the requirement of having externally visible global
functions for the hash/compare functions as well (which was ugly
anyway).
Comments?
Well, it looks nicer than what's there currently. As the element
functions now are scoped and normal member functions, they should be named
with lower case characters of course. I do like that the hash table would
then only have one real template argument; the Allocator, well, no better
idea comes to my mind.
Yeah. Updated patch below, all users converted. I probably missed
to revert some of the globalizations of hash/compare fns.
Comments?
Richard.
Index: gcc/hash-table.h
===
*** gcc/hash-table.h.orig 2012-08-15 15:39:34.0 +0200
--- gcc/hash-table.h2012-08-15 16:17:06.613628039 +0200
*** xcallocator Type::data_free (Type *mem
*** 83,127
}
- /* A common function for hashing a CANDIDATE typed pointer. */
-
template typename Element
! inline hashval_t
! typed_pointer_hash (const Element *candidate)
{
! /* This is a really poor hash function, but it is what the current code
uses,
! so I am reusing it to avoid an additional axis in testing. */
! return (hashval_t) ((intptr_t)candidate 3);
! }
- /* A common function for comparing an EXISTING and CANDIDATE typed pointers
-for equality. */
template typename Element
! inline int
! typed_pointer_equal (const Element *existing, const Element * candidate)
{
! return existing == candidate;
! }
! /* A common function for doing nothing on removing a RETIRED slot. */
template typename Element
! inline void
! typed_null_remove (Element *retired ATTRIBUTE_UNUSED)
{
}
-
- /* A common function for using free on removing a RETIRED slot. */
-
template typename Element
! inline void
! typed_free_remove (Element *retired)
{
! free (retired);
}
--- 83,132
}
template typename Element
! class typed_free_remove
{
! public:
! static inline void remove (Element *p) { free (p); }
! };
+ template typename Element
+ class typed_noop_remove
+ {
+ public:
+ static inline void remove (Element *) {}
+ };
+ /* Pointer hash. */
template typename Element
! class pointer_hash : public typed_noop_remove Element
{
! public:
! typedef Element Element_t;
+ static inline hashval_t
+ hash (const Element_t *);
! static inline int
! equal (const Element_t *existing, const Element_t * candidate);
! };
template typename Element
! inline hashval_t
! pointer_hashElement::hash (const Element_t *candidate)
{
+ /* This is a really poor hash function, but it is what the current code
uses,
+ so I am reusing it to avoid an additional axis in testing. */
+ return (hashval_t) ((intptr_t)candidate 3);
}
template typename Element
! inline int
! pointer_hashElement::equal (const Element_t *existing,
! const Element_t *candidate)
{
! return existing == candidate;
}
*** struct hash_table_control
*** 180,194
The table stores elements of type Element.
!It hashes elements with the Hash function.
The table currently works with relatively weak hash functions.
Use typed_pointer_hash Element when hashing pointers instead of
objects.
!It compares elements with the Equal function.
Two elements with the same hash may not be equal.
Use typed_pointer_equal Element when hashing pointers instead of
objects.
!It removes elements with the Remove function.
This feature is useful for freeing memory.
Use typed_null_remove Element when not freeing objects.
Use typed_free_remove Element when doing a simple object free.
--- 185,199
The table stores elements of type Element.
!It hashes elements with the hash function.
The table currently works with relatively weak hash functions.
Use typed_pointer_hash Element when hashing pointers instead of
objects.
!It compares elements with the equal function.
Two elements with the same hash may not be equal.
Use typed_pointer_equal Element when hashing pointers instead of
objects.
!It removes elements with the remove function.
This feature is useful for freeing memory.
Use typed_null_remove Element when not freeing objects.
Use typed_free_remove Element when doing a simple object free.
*** struct hash_table_control
*** 199,243
*/
Re: Merge C++ conversion into trunk (2/6 - VEC rewrite)
This implements the VEC re-write. See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for details. You didn't update the head comment in vec.h though, is that on purpose? -- Eric Botcazou
Re: Merge C++ conversion into trunk (2/6 - VEC rewrite)
On 12-08-15 10:44 , Eric Botcazou wrote: This implements the VEC re-write. See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for details. You didn't update the head comment in vec.h though, is that on purpose? Yes. I am updating it now that I'm *really* changing the interface. Diego.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Wed, 15 Aug 2012 14:23:37 +0200, Diego Novillo wrote: GDB folks, would it be hard to figure out that there is a single variant of the called function and trust the user that they are passing the right pointer value? It is a needless violation of C++ resolving rules. There are various easy way how to get it working (in .gdbinit or cc1-gdb.gdb define GDB function, define macro in GDB, use GDB python pretty printer instead (possibly even calling GCC inferior function) etc.). While I did not post such patch yet I would prefer to even forbid by default expressions like (gdb) print *0x1234567 and enforce (gdb) print *(int *)0x1234567 instead as the former syntax confuses people (as commonly seen on IRC), the same applies to calling functions without full debuginfo (they should require an explicit cast in GDB) etc. Regards, Jan
Re: Merge C++ conversion into trunk (0/6 - Overview)
Hi, On Wed, 15 Aug 2012, Jan Kratochvil wrote: It is a needless violation of C++ resolving rules. It's not needless as the examples here show. gdb is about helping people debug their stuff, not about language lawyering. There are various easy way how to get it working (in .gdbinit or cc1-gdb.gdb define GDB function, define macro in GDB, use GDB python pretty printer instead (possibly even calling GCC inferior function) We should define gdb macros for every not-overloaded function (which are _all_ GCC functions currently)? Doesn't scale. Ciao, Michael.
Re: Merge C++ conversion into trunk (0/6 - Overview)
Hi, On Wed, 15 Aug 2012 17:44:32 +0200, Michael Matz wrote: On Wed, 15 Aug 2012, Jan Kratochvil wrote: It's not needless as the examples here show. gdb is about helping people debug their stuff, not about language lawyering. In such case there should be a GDB setting for it as at least from my opinion it complicates the debugging. Then there may be different opinions what should be the default. Thanks, Jan
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 12-08-15 11:44 , Michael Matz wrote: Hi, On Wed, 15 Aug 2012, Jan Kratochvil wrote: It is a needless violation of C++ resolving rules. It's not needless as the examples here show. gdb is about helping people debug their stuff, not about language lawyering. Agreed. If I'm passing a numeric pointer constant, I'm already past the bleeding edge. I don't want gdb to get in the way. If the inferior call crashes, I get to keep both pieces. Thanks. Diego.
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 2012-08-15 07:29, Richard Guenther wrote: + typedef typename Element::Element_t Element_t; Can we use something less ugly than Element_t? Such as typedef typename Element::T T; ? Given that this name is scoped anyway... r~
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Wed, Aug 15, 2012 at 05:49:34PM +0200, Jan Kratochvil wrote: On Wed, 15 Aug 2012 17:44:32 +0200, Michael Matz wrote: On Wed, 15 Aug 2012, Jan Kratochvil wrote: It's not needless as the examples here show. gdb is about helping people debug their stuff, not about language lawyering. In such case there should be a GDB setting for it as at least from my opinion it complicates the debugging. Then there may be different opinions what should be the default. That would be fine for gcc development purposes. We could switch that mode on at the end of gcc .gdbinit . Jakub
Re: Merge C++ conversion into trunk (0/6 - Overview)
Diego == Diego Novillo dnovi...@google.com writes: Diego GDB folks, would it be hard to figure out that there is a single Diego variant of the called function and trust the user that they are Diego passing the right pointer value? I asked Keith to resurrect his patch for this. Tom
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Wed, Aug 15, 2012 at 12:53 PM, Tom Tromey tro...@redhat.com wrote: Diego == Diego Novillo dnovi...@google.com writes: Diego GDB folks, would it be hard to figure out that there is a single Diego variant of the called function and trust the user that they are Diego passing the right pointer value? I asked Keith to resurrect his patch for this. Since people are concerned about typing rules, would it be an option for GDB to allow people to input pointer literals with the p suffix (or 0p prefix instead of 0x)? -- Gaby
Re: Merge C++ conversion into trunk (0/6 - Overview)
Gaby == Gabriel Dos Reis g...@integrable-solutions.net writes: Tom I asked Keith to resurrect his patch for this. Gaby Since people are concerned about typing rules, would it Gaby be an option for GDB to allow people to input pointer Gaby literals with the p suffix (or 0p prefix instead of 0x)? I think on the whole I'd rather have one extension instead of two. It seems to me that the above would still require extensions in the overloading code, to deal with conversions from void*; or perhaps some magic pointer type. What I think Keith is going to do is take his patch, change it so that int-pointer conversions (if the int != 0) are given a different badness from other conversions (meaning that, in theory, this should only be chosen as a last resort, and shouldn't interfere with ordinary uses), and finally add a parameter so that the feature can be disabled. I hope this will be acceptable all around. Tom
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Wed, Aug 15, 2012 at 1:21 PM, Tom Tromey tro...@redhat.com wrote: Gaby == Gabriel Dos Reis g...@integrable-solutions.net writes: Tom I asked Keith to resurrect his patch for this. Gaby Since people are concerned about typing rules, would it Gaby be an option for GDB to allow people to input pointer Gaby literals with the p suffix (or 0p prefix instead of 0x)? I think on the whole I'd rather have one extension instead of two. That is a fair point :-) It seems to me that the above would still require extensions in the overloading code, to deal with conversions from void*; or perhaps some magic pointer type. What I think Keith is going to do is take his patch, change it so that int-pointer conversions (if the int != 0) are given a different badness from other conversions (meaning that, in theory, this should only be chosen as a last resort, and shouldn't interfere with ordinary uses), and finally add a parameter so that the feature can be disabled. I hope this will be acceptable all around. OK, that sounds reasonable. Thanks! -- Gaby
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 08/15/2012 06:00 PM, Diego Novillo wrote: On the switch to C++ as the build language for GCC ... Here are my results: 0:30 UTC - using C as the initial build language: http://gcc.gnu.org/ml/gcc-testresults/2012-08/msg01329.html and: 18:40 UTC - using C++ as the initial build language: http://gcc.gnu.org/ml/gcc-testresults/2012-08/msg01408.html both for x86_64-unknown-linux-gnu native (note: --with-build-config=bootstrap-lto). As far as I can, little difference. Congratulations, Diego and all the others who worked on this transition. Kind regards, -- Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290 Saturnushof 14, 3738 XG Maartensdijk, The Netherlands At home: http://moene.org/~toon/; weather: http://moene.org/~hirlam/ Progress of GNU Fortran: http://gcc.gnu.org/wiki/GFortran#news
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 8/15/12, Richard Guenther rguent...@suse.de wrote:
On Sun, 12 Aug 2012, Diego Novillo wrote:
This implements a new C++ hash table.
See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for
details.
Now as we see the result I'd have prefered a more C++-way instead
of making the conversion simple ...
Like
template typename Element
class hash_table
{
...
};
template typename Element
class pointer_hash
{
hashval_t hash ();
int equal (const Element *);
~Element ();
Element e;
};
and
/* Hash table with all same_succ entries. */
static hash_table pointer_hash struct same_succ_def same_succ_htab;
The existing way is simply too ugly ... so, why did you not invent
a nice C++ way? (please consider reverting the hashtable patch)
We are trying to balance several factors. Sometimes we're going
to pick multiple steps rather than a single step. In some cases,
as here, the intent was to make the client code changes minimal and
unsurprising while still getting the type safety and efficiency.
Other times, it may be a minor technical issue. In particular,
I prefer to avoid steps that might cause very poor matching of
lines in the diff. Others may choose differently. Together we
will learn where the tradeoffs lie.
More in a later response.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 8/15/12, Richard Guenther rguent...@suse.de wrote: On Wed, 15 Aug 2012, Michael Matz wrote: On Wed, 15 Aug 2012, Richard Guenther wrote: Like the following, only the coverage.c use is converted. I've never seen template function arguments anywhere and having to repeat them all over the place is really really ugly (yes, even if only in the implementation). This goes with static member functions and not wrapping any data. It goes away with the requirement of having externally visible global functions for the hash/compare functions as well (which was ugly anyway). Well, it looks nicer than what's there currently. As the element functions now are scoped and normal member functions, they should be named with lower case characters of course. I do like that the hash table would then only have one real template argument; the Allocator, well, no better idea comes to my mind. Yeah. Updated patch below, all users converted. I probably missed to revert some of the globalizations of hash/compare fns. Your conversion is a better abstraction, and something I'd wanted to get to eventually, so I support your conversion. BTW, the conding conventions say to put member function definitions out of line. -- Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On 8/15/12, Richard Henderson r...@redhat.com wrote: On 2012-08-15 07:29, Richard Guenther wrote: + typedef typename Element::Element_t Element_t; Can we use something less ugly than Element_t? Such as typedef typename Element::T T; ? Given that this name is scoped anyway... I do not much like _t names either. -- Lawrence Crowl
Re: Merge C++ conversion into trunk (4/6 - hash table rewrite)
On Wed, Aug 15, 2012 at 2:58 PM, Lawrence Crowl cr...@google.com wrote: I do not much like _t names either. Also, names ending in _t are reserved by POSIX if you #include sys/types.h. Though that may only apply to global names, not to types defined in classes or namespaces. Ian
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Mon, 13 Aug 2012, Lawrence Crowl wrote: On 8/13/12, Richard Guenther richard.guent...@gmail.com wrote: Increment/decrement operations did not exist, please do not add them at this point. Note that I have also added +=, -= and *= operations. Having them has three advantages. First, it matches expectations on what numeric types allow. Second, it results in more concise code. Third, it results in potentially faster code. I think we should be able to use those operators. When I run through changing call sites, I really want to change the sites to the final form, not do two passes. Ok. Thanks, Richard.
Re: Merge C++ conversion into trunk (3/6 - gengtype C++ support)
Hello Diego,
Just some minor comments.
Diego Novillo dnovi...@google.com a écrit:
[...]
+@section User-provided marking routines for template types
+When a template type @code{TP} is marked with @code{GTY}, all
+instances of that type are considered user-provided types. This means
+that the individual instances of @code{TP} do not need to marked with
s/to marked/to be marked/
+@code{GTY}. The user needs to provide template functions to mark all
+the fields of the type.
+
+The following code snippets represent all the functions that need to
+be provided. Note that type @code{TP} may reference to more than one
+type. In these snippets, there is only one type @code{T}, but there
+could be more.
+
+@smallexample
+templatetypename T
+void gt_ggc_mx (TPT *tp)
+@{
Just for my education, for the marking routines in general why having
the parameter tp be a pointer, rather than TPT tp ?
+ extern void gt_ggc_mx (T);
+
+ /* This marks field 'fld' of type 'T'. */
+ gt_ggc_mx (tp-fld);
+@}
[...]
--
Dodji
Re: Merge C++ conversion into trunk (3/6 - gengtype C++ support)
On 12-08-14 01:39 , Laurynas Biveinis wrote:
(walk_type): Set D-IN_PTR_FILED when walking a TYPE_POINTER.
FIELD
Done.
+fields is completely handled by user-provided routines. Section
+@ref{User GC} for details on what functions need to be provided.
See Section ... ?
Done.
+code is generated. For these types, the user is required to provide
+three functions: one to act as a marker for garbage collection, and
+two functions to act as marker and pointer walking for pre-compiled
+headers.
s/walking/walker ?
Done.
+In general, each marker @code{M} should call @code{M} for every
+pointer field in the structure. Fields that are not allocated in GC
+or are not pointers can be ignored.
must be ignored
Done.
+create_user_defined_type (const char *type_name, struct fileloc *pos)
...
+ template by preteding that each type is a field of TY. This is needed to
pretending
Done.
@@ -548,20 +603,30 @@ resolve_typedef (const char *s, struct fileloc *pos)
for (p = typedefs; p != NULL; p = p-next)
if (strcmp (p-name, s) == 0)
return p-type;
- error_at_line (pos, unidentified type `%s', s);
- return scalar_nonchar; /* treat as int */
+
+ /* If we did not find a typedef registered, assume this is a name
+ for a user-defined type which will need to provide its own
+ marking functions.
+
+ FIXME cxx-conversion. Emit an error once explicit annotations
+ for marking user types are implemented. */
+ return create_user_defined_type (s, pos);
Are explicit annotations for marking user types referring to GTY((user))?
Actually, the comment is wrong. When we don't recognize the type, we
simply consider this type as implicitly user-defined.
+static const char *
+filter_type_name (const char *type_name)
+{
Maybe this function should return const-less char *? The casts to cast
the const away for freeing it look a bit awkward.
Done.
+
+/* User-callable entry point for marking string X. */
points
Done.
+
+/* User-callable entry point for marking string X. */
points
Done.
--- a/gcc/stringpool.c
+++ b/gcc/stringpool.c
@@ -49,7 +49,7 @@ static const char digit_vector[] = {
struct ht *ident_hash;
-static hashnode alloc_node (hash_table *);
+static hashnode alloc_node (cpp_hash_table *);
static int mark_ident (struct cpp_reader *, hashnode, const void *);
static void *
@@ -70,7 +70,7 @@ init_stringpool (void)
/* Allocate a hash node. */
static hashnode
-alloc_node (hash_table *table ATTRIBUTE_UNUSED)
+alloc_node (cpp_hash_table *table ATTRIBUTE_UNUSED)
{
return GCC_IDENT_TO_HT_IDENT (make_node (IDENTIFIER_NODE));
}
These changes are not in the ChangeLog, are they intentional?
Sorry. They belong to the hash table patch. Both patches touched the
same file and I missed splitting these hunks.
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
+/* Garbage collection support for edge_def. */
+
+extern void gt_ggc_mx (tree);
+extern void gt_ggc_mx (gimple);
+extern void gt_ggc_mx (rtx);
+extern void gt_ggc_mx (basic_block);
+/* PCH support for edge_def. */
+
+extern void gt_pch_nx (tree);
+extern void gt_pch_nx (gimple);
+extern void gt_pch_nx (rtx);
+extern void gt_pch_nx (basic_block);
I wonder if these externs can be avoided by including gtype-desc.h. I
realize that gtype-desc.h declares a lot of stuff, but if tree-cfg.c
already declares GC roots, then it already should be pulling that
header in in through gt-tree-cfg.h.
Not really. These are never really defined anywhere. They are emitted
in gtype-desc.c, but we never emit prototypes for them. Going down that
road ends up in turning gengtype upside-down.
Diego.
Re: Merge C++ conversion into trunk (3/6 - gengtype C++ support)
On 12-08-14 04:38 , Dodji Seketeli wrote:
Hello Diego,
Just some minor comments.
Diego Novillo dnovi...@google.com a écrit:
[...]
+@section User-provided marking routines for template types
+When a template type @code{TP} is marked with @code{GTY}, all
+instances of that type are considered user-provided types. This means
+that the individual instances of @code{TP} do not need to marked with
s/to marked/to be marked/
Done.
+@code{GTY}. The user needs to provide template functions to mark all
+the fields of the type.
+
+The following code snippets represent all the functions that need to
+be provided. Note that type @code{TP} may reference to more than one
+type. In these snippets, there is only one type @code{T}, but there
+could be more.
+
+@smallexample
+templatetypename T
+void gt_ggc_mx (TPT *tp)
+@{
Just for my education, for the marking routines in general why having
the parameter tp be a pointer, rather than TPT tp ?
It would be better, yes. But since gengtype does not generate files
that include the proper headers, we can't. We can get away with forward
declaring the struct names and passing pointers around, but the minute
you add a type reference, all hell breaks loose.
Diego.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 12-08-14 09:48 , Diego Novillo wrote: This merge touches several files, so I'm thinking that the best time is going to be on Thu 16/Aug around 2:00 GMT. So, the fixes I needed from Lawrence are already in so we can proceed with the merge. I'll commit the merge tonight at ~2:00 GMT. After the merge is in, I will send an announcement and request major branch merges to wait for another 24 hrs to allow testers the chance to pick up this merge. Thanks. Diego.
Re: Merge C++ conversion into trunk (1/6 - Configury)
On Mon, Aug 13, 2012 at 2:34 AM, Diego Novillo dnovi...@google.com wrote: On 12-08-12 16:16 , Steven Bosscher wrote: On Sun, Aug 12, 2012 at 10:09 PM, Diego Novillo dnovi...@google.com wrote: This patch implements the configuration changes needed to bootstrap with a C++ compiler by default. Hi, Is it possible to add -fno-rtti to the default CXX_FLAGS, and remove it if necessary? I suppose, but I don't recall what the consensus was with rtti. I did not follow the coding conventions discussion very closely. The coding conventions say: Run-time type information (RTTI) is permitted when certain non-default --enable-checking options are enabled, so as to allow checkers to report dynamic types. However, by default, RTTI is not permitted and the compiler must build cleanly with -fno-rtti. Ciao! Steven
Re: Merge C++ conversion into trunk (1/6 - Configury)
On 12-08-13 02:51 , Steven Bosscher wrote: On Mon, Aug 13, 2012 at 2:34 AM, Diego Novillo dnovi...@google.com wrote: On 12-08-12 16:16 , Steven Bosscher wrote: On Sun, Aug 12, 2012 at 10:09 PM, Diego Novillo dnovi...@google.com wrote: This patch implements the configuration changes needed to bootstrap with a C++ compiler by default. Hi, Is it possible to add -fno-rtti to the default CXX_FLAGS, and remove it if necessary? I suppose, but I don't recall what the consensus was with rtti. I did not follow the coding conventions discussion very closely. The coding conventions say: Run-time type information (RTTI) is permitted when certain non-default --enable-checking options are enabled, so as to allow checkers to report dynamic types. However, by default, RTTI is not permitted and the compiler must build cleanly with -fno-rtti. Thanks. We actually already build with -fno-rtti. So, no changes are needed. Diego.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Sun, Aug 12, 2012 at 10:04 PM, Diego Novillo dnovi...@google.com wrote: I will be sending 6 patches that implement all the changes we have been making on the cxx-conversion branch. As described in http://gcc.gnu.org/ml/gcc/2012-08/msg00015.html, these patches change the default bootstrap process so that stage 1 always builds with a C++ compiler. Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. I have split the merge in 6 main patches. I will send these patches to the respective maintainers and gcc-patches. Please remember that the patches conform to the new C++ coding guidelines (http://gcc.gnu.org/codingconventions.html#Cxx_Conventions): 1- Configuration changes. 2- Re-write of VEC. 3- Re-write of gengtype to support C++ templates and user-provided marking functions. 4- New hash table class. 5- Re-write double_int. 6- Implement tree macros as inline functions so they can be called from gdb. As discussed before, several of these patches do not fully change the call sites to use the new APIs. We will do this change once the branch has been merged into trunk. Otherwise, the branch becomes a maintenance nightmare (despite not having changed many caller sites we were already starting to run into maintenance problems). As I understand only 1. to 3. were kind-of required for the merge, all other changes are a bonus at this time and should be delayed IMHO (thus not merged with this batch). I also understand that you will, quickly after merging 1. to 3. convert all VEC users and remove the old interface. This should be done before any of 4. - 6. is merged as generally we don't want the half-converted to persist, nor have multiple such half-conversions at the same time. I also understand that the merge of 1. to 3. will be followed by the promised gengtype improvements and re-organizations. Thus, please to the first C++ things very well. Thanks, Richard. For those who would like to build the conversion, you can either checkout the branch from SVN (svn://gcc.gnu.org/gcc/branches/cxx-conversion) or get the merged trunk I have in the git repo (branch dnovillo/cxx-conversion). The bootstrap changes have already been tested on a wide range of targets (http://gcc.gnu.org/wiki/CppBuildStatus). Additionally, I have tested the merged trunk on: x86_64-unknown-linux-gnu, mips64el-unknown-linux-gnu, powerpc64-unknown-linux-gnu, i686-pc-linux-gnu, and ia64-unknown-linux-gnu. Thanks. Diego.
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Sun, Aug 12, 2012 at 11:30 PM, Marc Glisse marc.gli...@inria.fr wrote:
On Sun, 12 Aug 2012, Diego Novillo wrote:
This implements the double_int rewrite.
See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for
details.
Diego.
I am taking it as a chance to ask a couple questions about the coding
conventions.
2012-08-12 Lawrence Crowl cr...@google.com
* hash-table.h
(typedef double_int): Change to struct (POD).
(double_int::make): New overloads for int to double-int
conversion.
Isn't that double_int::from_* now?
+typedef struct double_int
{
[...]
} double_int;
Does the coding convention say something about this verbosity?
+ HOST_WIDE_INT to_signed () const;
+ unsigned HOST_WIDE_INT to_unsigned () const;
+
+ /* Conversion query functions. */
+
+ bool fits_unsigned() const;
+ bool fits_signed() const;
Space before the parentheses or not?
+inline double_int
+double_int::operator ++ ()
+{
+ *this + double_int_one;
*this += double_int_one;
would be less confusing.
Increment/decrement operations did not exist, please do not add them
at this point.
Richard.
+ return *this;
+}
--
Marc Glisse
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Sun, Aug 12, 2012 at 11:30:59PM +0200, Marc Glisse wrote:
+inline double_int
+double_int::operator ++ ()
+{
+ *this + double_int_one;
*this += double_int_one;
would be less confusing.
Do you mean that *this + double_int_one; alone also works, just is
confusing? That would mean operator+ has side-effects, right?
Jakub
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Mon, 13 Aug 2012, Jakub Jelinek wrote:
On Sun, Aug 12, 2012 at 11:30:59PM +0200, Marc Glisse wrote:
+inline double_int
+double_int::operator ++ ()
+{
+ *this + double_int_one;
*this += double_int_one;
would be less confusing.
Do you mean that *this + double_int_one; alone also works, just is
confusing? That would mean operator+ has side-effects, right?
It works in that it compiles. It is confusing because the addition is
dead code and thus operator++ is a nop. Sorry for my confusing euphemism,
I should have called it a bug. operator+ has no side-effects AFAICS.
Note that there are many obvious places where this operator can be used:
varasm.c: i = double_int_add (i, double_int_one);
tree-vrp.c: prod2h = double_int_add (prod2h, double_int_one);
tree-ssa-loop-niter.c:bound = double_int_add (bound, double_int_one);
tree-ssa-loop-niter.c: *nit = double_int_add (*nit, double_int_one);
tree-ssa-loop-ivopts.c:max_niter = double_int_add (max_niter,
double_int_one);
gimple-fold.c:index = double_int_add (index, double_int_one);
etc.
As a side note, I don't usually like making operator+ a member function.
It doesn't matter when there is no implicit conversion, but if we ever add
them, it will make addition non-symmetric.
--
Marc Glisse
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Mon, Aug 13, 2012 at 5:32 AM, Marc Glisse marc.gli...@inria.fr wrote:
On Mon, 13 Aug 2012, Jakub Jelinek wrote:
On Sun, Aug 12, 2012 at 11:30:59PM +0200, Marc Glisse wrote:
+inline double_int
+double_int::operator ++ ()
+{
+ *this + double_int_one;
*this += double_int_one;
would be less confusing.
Do you mean that *this + double_int_one; alone also works, just is
confusing? That would mean operator+ has side-effects, right?
It works in that it compiles. It is confusing because the addition is dead
code and thus operator++ is a nop. Sorry for my confusing euphemism, I
should have called it a bug. operator+ has no side-effects AFAICS.
yes, it is just as confusing and a bug as
2.3 + 1;
is in plain C.
Note that there are many obvious places where this operator can be used:
varasm.c: i = double_int_add (i, double_int_one);
tree-vrp.c: prod2h = double_int_add (prod2h, double_int_one);
tree-ssa-loop-niter.c:bound = double_int_add (bound, double_int_one);
tree-ssa-loop-niter.c: *nit = double_int_add (*nit, double_int_one);
tree-ssa-loop-ivopts.c:max_niter = double_int_add (max_niter,
double_int_one);
gimple-fold.c:index = double_int_add (index, double_int_one);
etc.
As a side note, I don't usually like making operator+ a member function. It
doesn't matter when there is no implicit conversion, but if we ever add
them, it will make addition non-symmetric.
As not everybody is familiar with C++ litotes, let me expand on this. I believe
you are not opposing overloading operator+ on double_int. You are objecting
to its implementation being defined as a member function. That is you would be
perfectly fine with operator+ defined as a free function, e.g. not a
member function.
-- Gaby
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 12-08-13 05:37 , Richard Guenther wrote: On Sun, Aug 12, 2012 at 10:04 PM, Diego Novillo dnovi...@google.com wrote: I will be sending 6 patches that implement all the changes we have been making on the cxx-conversion branch. As described in http://gcc.gnu.org/ml/gcc/2012-08/msg00015.html, these patches change the default bootstrap process so that stage 1 always builds with a C++ compiler. Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. I have split the merge in 6 main patches. I will send these patches to the respective maintainers and gcc-patches. Please remember that the patches conform to the new C++ coding guidelines (http://gcc.gnu.org/codingconventions.html#Cxx_Conventions): 1- Configuration changes. 2- Re-write of VEC. 3- Re-write of gengtype to support C++ templates and user-provided marking functions. 4- New hash table class. 5- Re-write double_int. 6- Implement tree macros as inline functions so they can be called from gdb. As discussed before, several of these patches do not fully change the call sites to use the new APIs. We will do this change once the branch has been merged into trunk. Otherwise, the branch becomes a maintenance nightmare (despite not having changed many caller sites we were already starting to run into maintenance problems). As I understand only 1. to 3. were kind-of required for the merge, all other changes are a bonus at this time and should be delayed IMHO (thus not merged with this batch). Both #4 and #5 have the same issues as #3 (VEC). What remains to be done is update a whole swath of user code. This is hard to maintain in the branch and needs to be done during stage 1. The change in #6 is similarly ready, so delaying it makes no sense. What I can do is merge #1-#3 as one rev and merge the others as 3 separate revisions. I also understand that you will, quickly after merging 1. to 3. convert all VEC users and remove the old interface. This should be done before any of 4. - 6. is merged as generally we don't want the half-converted to persist, nor have multiple such half-conversions at the same time. Yes, there will be no half conversions. We are committed to continue making changes in this area. Diego.
Re: Merge C++ conversion into trunk (2/6 - VEC rewrite)
On 12-08-13 05:39 , Richard Guenther wrote: It's an odd thing that you need to touch code replacing - with . (yes, it's due to the use of references) but not at the same time convert those places to the new VEC interface. Yes. I hated this aspect of the initial conversion. It caused many merge problems. Unfortunately, I could not escape it. Diego.
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On 8/13/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Aug 13, 2012 Marc Glisse marc.gli...@inria.fr wrote:
On Mon, 13 Aug 2012, Jakub Jelinek wrote:
On Sun, Aug 12, 2012 at 11:30:59PM +0200, Marc Glisse wrote:
+inline double_int
+double_int::operator ++ ()
+{
+ *this + double_int_one;
*this += double_int_one;
would be less confusing.
Do you mean that *this + double_int_one; alone also works,
just is confusing? That would mean operator+ has side-effects,
right?
It works in that it compiles. It is confusing because the
addition is dead code and thus operator++ is a nop. Sorry for
my confusing euphemism, I should have called it a bug. operator+
has no side-effects AFAICS.
yes, it is just as confusing and a bug as
2.3 + 1;
is in plain C.
Yes, it is a bug. It's a bit disturbing that it wasn't caught
in bootstrap.
Note that there are many obvious places where this operator can be used:
varasm.c: i = double_int_add (i, double_int_one);
tree-vrp.c: prod2h = double_int_add (prod2h, double_int_one);
tree-ssa-loop-niter.c:bound = double_int_add (bound, double_int_one);
tree-ssa-loop-niter.c: *nit = double_int_add (*nit, double_int_one);
tree-ssa-loop-ivopts.c:max_niter = double_int_add (max_niter,
double_int_one);
gimple-fold.c:index = double_int_add (index, double_int_one);
etc.
As a side note, I don't usually like making operator+ a
member function. It doesn't matter when there is no implicit
conversion, but if we ever add them, it will make addition
non-symmetric.
As not everybody is familiar with C++ litotes, let me expand
on this. I believe you are not opposing overloading operator+ on
double_int. You are objecting to its implementation being defined
as a member function. That is you would be perfectly fine with
operator+ defined as a free function, e.g. not a member function.
In the absence of symmetric overloading, the code is slightly
cleaner with operators as member functions. It is easy to change
should we need symmetric overloads because, for the most part,
the change would have no effect on client code.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On 8/12/12, Marc Glisse marc.gli...@inria.fr wrote:
On Sun, 12 Aug 2012, Diego Novillo wrote:
This implements the double_int rewrite.
See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for
details.
I am taking it as a chance to ask a couple questions about the coding
conventions.
2012-08-12 Lawrence Crowl cr...@google.com
* hash-table.h
(typedef double_int): Change to struct (POD).
(double_int::make): New overloads for int to double-int conversion.
Isn't that double_int::from_* now?
Yes.
+typedef struct double_int
{
[...]
} double_int;
Does the coding convention say something about this verbosity?
No. It helps to have it in code that is compiled by both C and C++.
In this case, it will only be compiled by C++ and the verbosity
is unnecessary. I left the verbosity as it was to help keep the
diff synchronized. I certainly don't object to a cleanup pass for
this kind of stuff.
+ HOST_WIDE_INT to_signed () const;
+ unsigned HOST_WIDE_INT to_unsigned () const;
+
+ /* Conversion query functions. */
+
+ bool fits_unsigned() const;
+ bool fits_signed() const;
Space before the parentheses or not?
Space. Sorry, gcc is the only coding convention I've used that
requires the space. My fingers sometimes forget.
--
Lawrence Crowl
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On 8/13/12, Richard Guenther richard.guent...@gmail.com wrote: Increment/decrement operations did not exist, please do not add them at this point. Note that I have also added +=, -= and *= operations. Having them has three advantages. First, it matches expectations on what numeric types allow. Second, it results in more concise code. Third, it results in potentially faster code. I think we should be able to use those operators. When I run through changing call sites, I really want to change the sites to the final form, not do two passes. -- Lawrence Crowl
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Mon, Aug 13, 2012 at 5:41 AM, Richard Guenther richard.guent...@gmail.com wrote: *this += double_int_one; would be less confusing. Increment/decrement operations did not exist, please do not add them at this point. But they are going to be used when the call-sites are converted. There is no point in leaving them out now. Diego.
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On 08/13/2012 01:22 PM, Lawrence Crowl wrote: yes, it is just as confusing and a bug as 2.3 + 1; is in plain C. Yes, it is a bug. It's a bit disturbing that it wasn't caught in bootstrap. You'll recall that I pointed it out last time around as well. r~
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On 8/13/12, Richard Henderson r...@redhat.com wrote: On 08/13/2012 01:22 PM, Lawrence Crowl wrote: yes, it is just as confusing and a bug as 2.3 + 1; is in plain C. Yes, it is a bug. It's a bit disturbing that it wasn't caught in bootstrap. You'll recall that I pointed it out last time around as well. My memory must be losing bits. -- Lawrence Crowl
Re: Merge C++ conversion into trunk (3/6 - gengtype C++ support)
Diego -
I have some relatively minor comments. In general, the these changes
and the adjusted GTY doc intro blurb you sent in the other email are
OK with the minor comments addressed and if no objections from global
maintainers.
(walk_type): Set D-IN_PTR_FILED when walking a TYPE_POINTER.
FIELD
+fields is completely handled by user-provided routines. Section
+@ref{User GC} for details on what functions need to be provided.
See Section ... ?
+code is generated. For these types, the user is required to provide
+three functions: one to act as a marker for garbage collection, and
+two functions to act as marker and pointer walking for pre-compiled
+headers.
s/walking/walker ?
+In general, each marker @code{M} should call @code{M} for every
+pointer field in the structure. Fields that are not allocated in GC
+or are not pointers can be ignored.
must be ignored
+create_user_defined_type (const char *type_name, struct fileloc *pos)
...
+ template by preteding that each type is a field of TY. This is needed
to
pretending
@@ -548,20 +603,30 @@ resolve_typedef (const char *s, struct fileloc *pos)
for (p = typedefs; p != NULL; p = p-next)
if (strcmp (p-name, s) == 0)
return p-type;
- error_at_line (pos, unidentified type `%s', s);
- return scalar_nonchar; /* treat as int */
+
+ /* If we did not find a typedef registered, assume this is a name
+ for a user-defined type which will need to provide its own
+ marking functions.
+
+ FIXME cxx-conversion. Emit an error once explicit annotations
+ for marking user types are implemented. */
+ return create_user_defined_type (s, pos);
Are explicit annotations for marking user types referring to GTY((user))?
+static const char *
+filter_type_name (const char *type_name)
+{
Maybe this function should return const-less char *? The casts to cast
the const away for freeing it look a bit awkward.
+ const char *id_for_tag = filter_type_name (s-u.s.tag);
+ oprintf (of, gt_%sx_%s, prefix, id_for_tag);
+ if (id_for_tag != s-u.s.tag)
+ free (CONST_CAST(char *, id_for_tag));
For example, here.
diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index d3d186d..f43d0c2 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -1441,6 +1441,26 @@ gt_ggc_m_S (const void *p)
return;
}
+
+/* User-callable entry point for marking string X. */
points
+
+void
+gt_ggc_mx (const char * x)
+{
+ gt_ggc_m_S (x);
+}
+
+void
+gt_ggc_mx (unsigned char * x)
+{
+ gt_ggc_m_S (x);
+}
+
+void
+gt_ggc_mx (unsigned char x ATTRIBUTE_UNUSED)
+{
+}
+
/* If P is not marked, marks it and return false. Otherwise return true.
P must have been allocated by the GC allocator; it mustn't point to
static objects, stack variables, or memory allocated with malloc. */
diff --git a/gcc/ggc-zone.c b/gcc/ggc-zone.c
index baf8076..3fe0dd2 100644
--- a/gcc/ggc-zone.c
+++ b/gcc/ggc-zone.c
@@ -1508,6 +1508,26 @@ gt_ggc_m_S (const void *p)
ggc_set_mark (p);
}
+
+/* User-callable entry point for marking string X. */
points
diff --git a/gcc/stringpool.c b/gcc/stringpool.c
index 747db17..281e550 100644
--- a/gcc/stringpool.c
+++ b/gcc/stringpool.c
@@ -49,7 +49,7 @@ static const char digit_vector[] = {
struct ht *ident_hash;
-static hashnode alloc_node (hash_table *);
+static hashnode alloc_node (cpp_hash_table *);
static int mark_ident (struct cpp_reader *, hashnode, const void *);
static void *
@@ -70,7 +70,7 @@ init_stringpool (void)
/* Allocate a hash node. */
static hashnode
-alloc_node (hash_table *table ATTRIBUTE_UNUSED)
+alloc_node (cpp_hash_table *table ATTRIBUTE_UNUSED)
{
return GCC_IDENT_TO_HT_IDENT (make_node (IDENTIFIER_NODE));
}
These changes are not in the ChangeLog, are they intentional?
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
+/* Garbage collection support for edge_def. */
+
+extern void gt_ggc_mx (tree);
+extern void gt_ggc_mx (gimple);
+extern void gt_ggc_mx (rtx);
+extern void gt_ggc_mx (basic_block);
+/* PCH support for edge_def. */
+
+extern void gt_pch_nx (tree);
+extern void gt_pch_nx (gimple);
+extern void gt_pch_nx (rtx);
+extern void gt_pch_nx (basic_block);
I wonder if these externs can be avoided by including gtype-desc.h. I
realize that gtype-desc.h declares a lot of stuff, but if tree-cfg.c
already declares GC roots, then it already should be pulling that
header in in through gt-tree-cfg.h.
--
Laurynas
Merge C++ conversion into trunk (0/6 - Overview)
I will be sending 6 patches that implement all the changes we have been making on the cxx-conversion branch. As described in http://gcc.gnu.org/ml/gcc/2012-08/msg00015.html, these patches change the default bootstrap process so that stage 1 always builds with a C++ compiler. Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. I have split the merge in 6 main patches. I will send these patches to the respective maintainers and gcc-patches. Please remember that the patches conform to the new C++ coding guidelines (http://gcc.gnu.org/codingconventions.html#Cxx_Conventions): 1- Configuration changes. 2- Re-write of VEC. 3- Re-write of gengtype to support C++ templates and user-provided marking functions. 4- New hash table class. 5- Re-write double_int. 6- Implement tree macros as inline functions so they can be called from gdb. As discussed before, several of these patches do not fully change the call sites to use the new APIs. We will do this change once the branch has been merged into trunk. Otherwise, the branch becomes a maintenance nightmare (despite not having changed many caller sites we were already starting to run into maintenance problems). For those who would like to build the conversion, you can either checkout the branch from SVN (svn://gcc.gnu.org/gcc/branches/cxx-conversion) or get the merged trunk I have in the git repo (branch dnovillo/cxx-conversion). The bootstrap changes have already been tested on a wide range of targets (http://gcc.gnu.org/wiki/CppBuildStatus). Additionally, I have tested the merged trunk on: x86_64-unknown-linux-gnu, mips64el-unknown-linux-gnu, powerpc64-unknown-linux-gnu, i686-pc-linux-gnu, and ia64-unknown-linux-gnu. Thanks. Diego.
Merge C++ conversion into trunk (1/6 - Configury)
This patch implements the configuration changes needed to bootstrap with a C++ compiler by default. See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for details. Diego. ChangeLog 2012-08-12 Diego Novillo dnovi...@google.com * Makefile.in (CXX_FOR_BUILD): Define. (BUILD_CXX_FLAGS): Define (COMPILER_FOR_BUILD): Set to CXX_FOR_BUILD if building with C++. (LINKER_FOR_BUILD): Likewise. (BUILD_COMPILERFLAGS): Set to BUILD_CXXFLAGS if building with C++. (BUILD_LINKERFLAGS): Likewise. * Makefile.tpl (STAGE[+id+]_CXXFLAGS): Remove POSTSTAGE1_CONFIGURE_FLAGS. * Makefile.in: Regenerate. * configure.ac (ENABLE_BUILD_WITH_CXX): Remove. Update all users. * configure: Regenerate. gcc/ChangeLog * configure.ac (BUILD_CXXFLAGS): Define. * Makefile.in (BUILD_CXXFLAGS): Use it. * configure: Regenerate. diff --git a/Makefile.in b/Makefile.in index 346c4bf..0d25666 100644 --- a/Makefile.in +++ b/Makefile.in @@ -420,7 +420,6 @@ TFLAGS = STAGE_CFLAGS = $(BOOT_CFLAGS) STAGE_TFLAGS = $(TFLAGS) STAGE_CONFIGURE_FLAGS=@stage2_werror_flag@ -POSTSTAGE1_CONFIGURE_FLAGS = @POSTSTAGE1_CONFIGURE_FLAGS@ # Defaults for stage 1; some are overridden below. @@ -431,10 +430,7 @@ STAGE1_CXXFLAGS = $(CXXFLAGS) STAGE1_CXXFLAGS = $(STAGE1_CFLAGS) @endif target-libstdc++-v3-bootstrap STAGE1_TFLAGS = $(STAGE_TFLAGS) -# STAGE1_CONFIGURE_FLAGS overridden below, so we can use -# POSTSTAGE1_CONFIGURE_FLAGS here. -STAGE1_CONFIGURE_FLAGS = \ - $(STAGE_CONFIGURE_FLAGS) $(POSTSTAGE1_CONFIGURE_FLAGS) +STAGE1_CONFIGURE_FLAGS = $(STAGE_CONFIGURE_FLAGS) # Defaults for stage 2; some are overridden below. STAGE2_CFLAGS = $(STAGE_CFLAGS) @@ -444,10 +440,7 @@ STAGE2_CXXFLAGS = $(CXXFLAGS) STAGE2_CXXFLAGS = $(STAGE2_CFLAGS) @endif target-libstdc++-v3-bootstrap STAGE2_TFLAGS = $(STAGE_TFLAGS) -# STAGE1_CONFIGURE_FLAGS overridden below, so we can use -# POSTSTAGE1_CONFIGURE_FLAGS here. -STAGE2_CONFIGURE_FLAGS = \ - $(STAGE_CONFIGURE_FLAGS) $(POSTSTAGE1_CONFIGURE_FLAGS) +STAGE2_CONFIGURE_FLAGS = $(STAGE_CONFIGURE_FLAGS) # Defaults for stage 3; some are overridden below. STAGE3_CFLAGS = $(STAGE_CFLAGS) @@ -457,10 +450,7 @@ STAGE3_CXXFLAGS = $(CXXFLAGS) STAGE3_CXXFLAGS = $(STAGE3_CFLAGS) @endif target-libstdc++-v3-bootstrap STAGE3_TFLAGS = $(STAGE_TFLAGS) -# STAGE1_CONFIGURE_FLAGS overridden below, so we can use -# POSTSTAGE1_CONFIGURE_FLAGS here. -STAGE3_CONFIGURE_FLAGS = \ - $(STAGE_CONFIGURE_FLAGS) $(POSTSTAGE1_CONFIGURE_FLAGS) +STAGE3_CONFIGURE_FLAGS = $(STAGE_CONFIGURE_FLAGS) # Defaults for stage 4; some are overridden below. STAGE4_CFLAGS = $(STAGE_CFLAGS) @@ -470,10 +460,7 @@ STAGE4_CXXFLAGS = $(CXXFLAGS) STAGE4_CXXFLAGS = $(STAGE4_CFLAGS) @endif target-libstdc++-v3-bootstrap STAGE4_TFLAGS = $(STAGE_TFLAGS) -# STAGE1_CONFIGURE_FLAGS overridden below, so we can use -# POSTSTAGE1_CONFIGURE_FLAGS here. -STAGE4_CONFIGURE_FLAGS = \ - $(STAGE_CONFIGURE_FLAGS) $(POSTSTAGE1_CONFIGURE_FLAGS) +STAGE4_CONFIGURE_FLAGS = $(STAGE_CONFIGURE_FLAGS) # Defaults for stage profile; some are overridden below. STAGEprofile_CFLAGS = $(STAGE_CFLAGS) @@ -483,10 +470,7 @@ STAGEprofile_CXXFLAGS = $(CXXFLAGS) STAGEprofile_CXXFLAGS = $(STAGEprofile_CFLAGS) @endif target-libstdc++-v3-bootstrap STAGEprofile_TFLAGS = $(STAGE_TFLAGS) -# STAGE1_CONFIGURE_FLAGS overridden below, so we can use -# POSTSTAGE1_CONFIGURE_FLAGS here. -STAGEprofile_CONFIGURE_FLAGS = \ - $(STAGE_CONFIGURE_FLAGS) $(POSTSTAGE1_CONFIGURE_FLAGS) +STAGEprofile_CONFIGURE_FLAGS = $(STAGE_CONFIGURE_FLAGS) # Defaults for stage feedback; some are overridden below. STAGEfeedback_CFLAGS = $(STAGE_CFLAGS) @@ -496,10 +480,7 @@ STAGEfeedback_CXXFLAGS = $(CXXFLAGS) STAGEfeedback_CXXFLAGS = $(STAGEfeedback_CFLAGS) @endif target-libstdc++-v3-bootstrap STAGEfeedback_TFLAGS = $(STAGE_TFLAGS) -# STAGE1_CONFIGURE_FLAGS overridden below, so we can use -# POSTSTAGE1_CONFIGURE_FLAGS here. -STAGEfeedback_CONFIGURE_FLAGS = \ - $(STAGE_CONFIGURE_FLAGS) $(POSTSTAGE1_CONFIGURE_FLAGS) +STAGEfeedback_CONFIGURE_FLAGS = $(STAGE_CONFIGURE_FLAGS) # Only build the C compiler for stage1, because that is the only one that @@ -517,9 +498,6 @@ STAGE1_LANGUAGES = @stage1_languages@ # the last argument when conflicting --enable arguments are passed. # * Likewise, we force-disable coverage flags, since the installed # compiler probably has never heard of them. -# * Don't remove this, because above we added -# POSTSTAGE1_CONFIGURE_FLAGS to STAGE_CONFIGURE_FLAGS, which -# we don't want for STAGE1_CONFIGURE_FLAGS. STAGE1_CONFIGURE_FLAGS = --disable-intermodule $(STAGE1_CHECKING) \ --disable-coverage --enable-languages=$(STAGE1_LANGUAGES) diff --git a/Makefile.tpl b/Makefile.tpl index 2573eee..f2c3f48 100644 --- a/Makefile.tpl +++ b/Makefile.tpl @@ -423,7 +423,6 @@ TFLAGS = STAGE_CFLAGS = $(BOOT_CFLAGS) STAGE_TFLAGS = $(TFLAGS)
Re: Merge C++ conversion into trunk (1/6 - Configury)
On Sun, Aug 12, 2012 at 10:09 PM, Diego Novillo dnovi...@google.com wrote: This patch implements the configuration changes needed to bootstrap with a C++ compiler by default. Hi, Is it possible to add -fno-rtti to the default CXX_FLAGS, and remove it if necessary? Ciao! Steven
Merge C++ conversion into trunk (6/6 - gdb tree macro support)
This implements support for calling tree macros from gdb.
See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for
details.
Diego.
2012-08-12 Lawrence Crowl cr...@google.com
* tree.h (tree_check): New.
(TREE_CHECK): Use inline function above instead of __extension__.
(tree_not_check): New.
(TREE_NOT_CHECK): Use inline function above instead of __extension__.
(tree_check2): New.
(TREE_CHECK2): Use inline function above instead of __extension__.
(tree_not_check2): New.
(TREE_NOT_CHECK2): Use inline function above instead of __extension__.
(tree_check3): New.
(TREE_CHECK3): Use inline function above instead of __extension__.
(tree_not_check3): New.
(TREE_NOT_CHECK3): Use inline function above instead of __extension__.
(tree_check4): New.
(TREE_CHECK4): Use inline function above instead of __extension__.
(tree_not_check4): New.
(TREE_NOT_CHECK4): Use inline function above instead of __extension__.
(tree_check5): New.
(TREE_CHECK5): Use inline function above instead of __extension__.
(tree_not_check5): New.
(TREE_NOT_CHECK5): Use inline function above instead of __extension__.
(contains_struct_check): New.
(CONTAINS_STRUCT_CHECK): Use inline function above instead of
__extension__.
(tree_class_check): New.
(TREE_CLASS_CHECK): Use inline function above instead of __extension__.
(tree_range_check): New.
(TREE_RANGE_CHECK): Use inline function above instead of __extension__.
(omp_clause_subcode_check): New.
(OMP_CLAUSE_SUBCODE_CHECK): Use inline function above instead of
__extension__.
(omp_clause_range_check): New.
(OMP_CLAUSE_RANGE_CHECK): Use inline function above instead of
__extension__.
(expr_check): New.
(EXPR_CHECK): Use inline function above instead of __extension__.
(non_type_check): New.
(NON_TYPE_CHECK): Use inline function above instead of __extension__.
(tree_vec_elt_check): New.
(TREE_VEC_ELT_CHECK): Use inline function above instead of
__extension__.
(omp_clause_elt_check): New.
(OMP_CLAUSE_ELT_CHECK): Use inline function above instead of
__extension__.
(tree_operand_check): New.
(TREE_OPERAND_CHECK): Use inline function above instead of
__extension__.
(tree_operand_check_code): New.
(TREE_OPERAND_CHECK_CODE): Use inline function above instead of
__extension__.
(TREE_CHAIN): Simplify implementation.
(TREE_TYPE): Simplify implementation.
(tree_operand_length): Move for compilation dependences.
* gdbinit.in: (macro define __FILE__): New.
(macro define __LINE__): New.
(skip tree.h): New.
* tree.h (tree_check): Change from template to const overload.
(tree_not_check): Likewise.
(tree_check2): Likewise.
(tree_not_check2): Likewise.
(tree_check3): Likewise.
(tree_not_check3): Likewise.
(tree_check4): Likewise.
(tree_not_check4): Likewise.
(tree_check5): Likewise.
(tree_not_check5): Likewise.
(contains_struct_check): Likewise.
(tree_class_check): Likewise.
(tree_range_check): Likewise.
(omp_clause_subcode_check): Likewise.
(omp_clause_range_check): Likewise.
(expr_check): Likewise.
(non_type_check): Likewise.
(tree_vec_elt_check): Likewise.
(omp_clause_elt_check): Likewise.
(tree_operand_check): Likewise.
(tree_operand_check_code): Likewise.
(tree_operand_length): Move to avoid a duplicate copy.
* gdbinit.in (set unwindonsignal on): New.
diff --git a/gcc/gdbinit.in b/gcc/gdbinit.in
index d1ae46d..858e490 100644
--- a/gcc/gdbinit.in
+++ b/gcc/gdbinit.in
@@ -182,6 +182,17 @@ document pbm
Dump the bitmap that is in $ as a comma-separated list of numbers.
end
+# Define some macros helpful to gdb when it is expanding macros.
+macro define __FILE__ gdb
+macro define __LINE__ 1
+
+# Skip all inline functions in tree.h.
+# These are used in accessor macros.
+skip tree.h
+
+# Gracefully handle aborts in functions used from gdb.
+set unwindonsignal on
+
# Put breakpoints at exit and fancy_abort in case abort is mapped
# to either fprintf/exit or fancy_abort.
b fancy_abort
diff --git a/gcc/tree.h b/gcc/tree.h
index cc49629..ba4a021 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -719,195 +719,80 @@ enum tree_node_structure_enum {
is accessed incorrectly. The macros die with a fatal error. */
#if defined ENABLE_TREE_CHECKING (GCC_VERSION = 2007)
-#define TREE_CHECK(T, CODE) __extension__ \
-({ __typeof (T) const __t = (T); \
-if (TREE_CODE (__t) != (CODE)) \
-
Re: Merge C++ conversion into trunk (6/6 - gdb tree macro support)
Diego Novillo dnovi...@google.com writes: diff --git a/gcc/gdbinit.in b/gcc/gdbinit.in index d1ae46d..858e490 100644 --- a/gcc/gdbinit.in +++ b/gcc/gdbinit.in @@ -182,6 +182,17 @@ document pbm Dump the bitmap that is in $ as a comma-separated list of numbers. end +# Define some macros helpful to gdb when it is expanding macros. +macro define __FILE__ gdb +macro define __LINE__ 1 + +# Skip all inline functions in tree.h. +# These are used in accessor macros. +skip tree.h This command has only been added to the current gdb release. Older versions will bail out at this point, so it should be moved down to the end of the file. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
Re: Merge C++ conversion into trunk (1/6 - Configury)
On Sun, 12 Aug 2012, Diego Novillo wrote: ChangeLog 2012-08-12 Diego Novillo dnovi...@google.com * Makefile.in (CXX_FOR_BUILD): Define. (BUILD_CXX_FLAGS): Define (COMPILER_FOR_BUILD): Set to CXX_FOR_BUILD if building with C++. (LINKER_FOR_BUILD): Likewise. (BUILD_COMPILERFLAGS): Set to BUILD_CXXFLAGS if building with C++. (BUILD_LINKERFLAGS): Likewise. * Makefile.tpl (STAGE[+id+]_CXXFLAGS): Remove POSTSTAGE1_CONFIGURE_FLAGS. * Makefile.in: Regenerate. * configure.ac (ENABLE_BUILD_WITH_CXX): Remove. Update all users. * configure: Regenerate. gcc/ChangeLog * configure.ac (BUILD_CXXFLAGS): Define. * Makefile.in (BUILD_CXXFLAGS): Use it. * configure: Regenerate. This changes gcc/doc/install.texi, but that change is not documented in any ChangeLog? Gerald
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Sun, 12 Aug 2012, Diego Novillo wrote: For those who would like to build the conversion, you can either checkout the branch from SVN (svn://gcc.gnu.org/gcc/branches/cxx-conversion) or get the merged trunk I have in the git repo (branch dnovillo/cxx-conversion). The bootstrap changes have already been tested on a wide range of targets (http://gcc.gnu.org/wiki/CppBuildStatus). Hello, as a matter of policy, is it required that non-gcc compilers can be used to initiate the bootstrap? In the table, clang seems to be the only other compiler that managed. IBM and Oracle both fail (the comment is not clear, but I think 12.3 also fails, just not exactly in the same way), and HP and Intel (to mention just a few) are not listed. -- Marc Glisse PS: I never know if it is better to trim Cc: lists in answers or let them grow...
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Sun, 12 Aug 2012, Diego Novillo wrote:
This implements the double_int rewrite.
See http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00711.html for
details.
Diego.
I am taking it as a chance to ask a couple questions about the coding
conventions.
2012-08-12 Lawrence Crowl cr...@google.com
* hash-table.h
(typedef double_int): Change to struct (POD).
(double_int::make): New overloads for int to double-int conversion.
Isn't that double_int::from_* now?
+typedef struct double_int
{
[...]
} double_int;
Does the coding convention say something about this verbosity?
+ HOST_WIDE_INT to_signed () const;
+ unsigned HOST_WIDE_INT to_unsigned () const;
+
+ /* Conversion query functions. */
+
+ bool fits_unsigned() const;
+ bool fits_signed() const;
Space before the parentheses or not?
+inline double_int
+double_int::operator ++ ()
+{
+ *this + double_int_one;
*this += double_int_one;
would be less confusing.
+ return *this;
+}
--
Marc Glisse
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Sun, Aug 12, 2012 at 1:04 PM, Diego Novillo dnovi...@google.com wrote: I will be sending 6 patches that implement all the changes we have been making on the cxx-conversion branch. As described in http://gcc.gnu.org/ml/gcc/2012-08/msg00015.html, these patches change the default bootstrap process so that stage 1 always builds with a C++ compiler. Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. I have split the merge in 6 main patches. I will send these patches to the respective maintainers and gcc-patches. Please remember that the patches conform to the new C++ coding guidelines (http://gcc.gnu.org/codingconventions.html#Cxx_Conventions): 1- Configuration changes. 2- Re-write of VEC. 3- Re-write of gengtype to support C++ templates and user-provided marking functions. 4- New hash table class. 5- Re-write double_int. 6- Implement tree macros as inline functions so they can be called from gdb. As discussed before, several of these patches do not fully change the call sites to use the new APIs. We will do this change once the branch has been merged into trunk. Otherwise, the branch becomes a maintenance nightmare (despite not having changed many caller sites we were already starting to run into maintenance problems). For those who would like to build the conversion, you can either checkout the branch from SVN (svn://gcc.gnu.org/gcc/branches/cxx-conversion) or get the merged trunk I have in the git repo (branch dnovillo/cxx-conversion). dnovillo/cxx-conversion git branch failed to bootstrap on Fedora 17 x86-64 when configured with --enable-languages=c,c++,fortran,java,lto,objc,obj-c++,go I got /export/gnu/import/git/gcc-x32/gcc/objc/objc-act.c: In function \u2018tree_node* objc_build_constructor(tree, vec_tconstructor_elt_d*)\u2019: /export/gnu/import/git/gcc-x32/gcc/objc/objc-act.c:3212:44: error: base operand of \u2018-\u2019 has non-pointer type \u2018constructor_elt_d\u2019 if (!VEC_index (constructor_elt, elts, 0)-index) -- H.J.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Sun, Aug 12, 2012 at 3:33 PM, H.J. Lu hjl.to...@gmail.com wrote: On Sun, Aug 12, 2012 at 1:04 PM, Diego Novillo dnovi...@google.com wrote: I will be sending 6 patches that implement all the changes we have been making on the cxx-conversion branch. As described in http://gcc.gnu.org/ml/gcc/2012-08/msg00015.html, these patches change the default bootstrap process so that stage 1 always builds with a C++ compiler. Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. I have split the merge in 6 main patches. I will send these patches to the respective maintainers and gcc-patches. Please remember that the patches conform to the new C++ coding guidelines (http://gcc.gnu.org/codingconventions.html#Cxx_Conventions): 1- Configuration changes. 2- Re-write of VEC. 3- Re-write of gengtype to support C++ templates and user-provided marking functions. 4- New hash table class. 5- Re-write double_int. 6- Implement tree macros as inline functions so they can be called from gdb. As discussed before, several of these patches do not fully change the call sites to use the new APIs. We will do this change once the branch has been merged into trunk. Otherwise, the branch becomes a maintenance nightmare (despite not having changed many caller sites we were already starting to run into maintenance problems). For those who would like to build the conversion, you can either checkout the branch from SVN (svn://gcc.gnu.org/gcc/branches/cxx-conversion) or get the merged trunk I have in the git repo (branch dnovillo/cxx-conversion). dnovillo/cxx-conversion git branch failed to bootstrap on Fedora 17 x86-64 when configured with --enable-languages=c,c++,fortran,java,lto,objc,obj-c++,go I got /export/gnu/import/git/gcc-x32/gcc/objc/objc-act.c: In function \u2018tree_node* objc_build_constructor(tree, vec_tconstructor_elt_d*)\u2019: /export/gnu/import/git/gcc-x32/gcc/objc/objc-act.c:3212:44: error: base operand of \u2018-\u2019 has non-pointer type \u2018constructor_elt_d\u2019 if (!VEC_index (constructor_elt, elts, 0)-index) This patch fixes the error: diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c index 5c924bf..caa16c7 100644 --- a/gcc/objc/objc-act.c +++ b/gcc/objc/objc-act.c @@ -3209,7 +3209,7 @@ objc_build_constructor (tree type, VEC(constructor_elt,gc) *elts) #ifdef OBJCPLUS /* Adjust for impedance mismatch. We should figure out how to build CONSTRUCTORs that consistently please both the C and C++ gods. */ - if (!VEC_index (constructor_elt, elts, 0)-index) + if (!VEC_index (constructor_elt, elts, 0).index) TREE_TYPE (constructor) = init_list_type_node; #endif -- H.J.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 12-08-12 18:38 , H.J. Lu wrote: On Sun, Aug 12, 2012 at 3:33 PM, H.J. Lu hjl.to...@gmail.com wrote: On Sun, Aug 12, 2012 at 1:04 PM, Diego Novillo dnovi...@google.com wrote: I will be sending 6 patches that implement all the changes we have been making on the cxx-conversion branch. As described in http://gcc.gnu.org/ml/gcc/2012-08/msg00015.html, these patches change the default bootstrap process so that stage 1 always builds with a C++ compiler. Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. I have split the merge in 6 main patches. I will send these patches to the respective maintainers and gcc-patches. Please remember that the patches conform to the new C++ coding guidelines (http://gcc.gnu.org/codingconventions.html#Cxx_Conventions): 1- Configuration changes. 2- Re-write of VEC. 3- Re-write of gengtype to support C++ templates and user-provided marking functions. 4- New hash table class. 5- Re-write double_int. 6- Implement tree macros as inline functions so they can be called from gdb. As discussed before, several of these patches do not fully change the call sites to use the new APIs. We will do this change once the branch has been merged into trunk. Otherwise, the branch becomes a maintenance nightmare (despite not having changed many caller sites we were already starting to run into maintenance problems). For those who would like to build the conversion, you can either checkout the branch from SVN (svn://gcc.gnu.org/gcc/branches/cxx-conversion) or get the merged trunk I have in the git repo (branch dnovillo/cxx-conversion). dnovillo/cxx-conversion git branch failed to bootstrap on Fedora 17 x86-64 when configured with --enable-languages=c,c++,fortran,java,lto,objc,obj-c++,go I got /export/gnu/import/git/gcc-x32/gcc/objc/objc-act.c: In function \u2018tree_node* objc_build_constructor(tree, vec_tconstructor_elt_d*)\u2019: /export/gnu/import/git/gcc-x32/gcc/objc/objc-act.c:3212:44: error: base operand of \u2018-\u2019 has non-pointer type \u2018constructor_elt_d\u2019 if (!VEC_index (constructor_elt, elts, 0)-index) This patch fixes the error: diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c index 5c924bf..caa16c7 100644 --- a/gcc/objc/objc-act.c +++ b/gcc/objc/objc-act.c @@ -3209,7 +3209,7 @@ objc_build_constructor (tree type, VEC(constructor_elt,gc) *elts) #ifdef OBJCPLUS /* Adjust for impedance mismatch. We should figure out how to build CONSTRUCTORs that consistently please both the C and C++ gods. */ - if (!VEC_index (constructor_elt, elts, 0)-index) + if (!VEC_index (constructor_elt, elts, 0).index) TREE_TYPE (constructor) = init_list_type_node; #endif Thanks. Missed this because --enable-languages=all does not enable obj-c++. Please commit to the branch. I'll update the git image. Diego.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 12-08-12 16:57 , Marc Glisse wrote: other compiler that managed. IBM and Oracle both fail (the comment is not clear, but I think 12.3 also fails, just not exactly in the same way), and HP and Intel (to mention just a few) are not listed. We should fix/workaround failing host C++ compilers similarly to how we deal with host C compilers. If you have access to any of the compilers you mention, could you try building the branch? Thanks. Diego.
Re: Merge C++ conversion into trunk (1/6 - Configury)
On 12-08-12 16:16 , Steven Bosscher wrote: On Sun, Aug 12, 2012 at 10:09 PM, Diego Novillo dnovi...@google.com wrote: This patch implements the configuration changes needed to bootstrap with a C++ compiler by default. Hi, Is it possible to add -fno-rtti to the default CXX_FLAGS, and remove it if necessary? I suppose, but I don't recall what the consensus was with rtti. I did not follow the coding conventions discussion very closely. Diego.
