Re: Use conditional casting with symtab_node
On Fri, Oct 12, 2012 at 4:22 AM, Richard Biener richard.guent...@gmail.com wrote: I also think that instead of if (cgraph_node *q = p-cast_to cgraph_node * ()) we want if ((q = cast_to cgraph_node * (p)) I see absolutely no good reason to make cast_to a member, given that the language has static_cast, const_cast and stuff. cast_to would simply be our equivalent to dynamic_cast within our OO model. Then I'd call it *_cast instead of cast_*, so, why not gcc_cast ? Or dyn_cast (). That way if ((q = dyn_cast function * (p)) This looks fine to me. Diego.
Re: Use conditional casting with symtab_node
On 10/12/12, Richard Biener richard.guent...@gmail.com wrote:
On Oct 11, 2012 Xinliang David Li davi...@google.com wrote:
On Oct 11, 2012 Lawrence Crowl cr...@googlers.com wrote:
On 10/10/12, Xinliang David Li davi...@google.com wrote:
In a different thread, I proposed the following alternative to
'try_xxx':
templatetypename T T* symbol::cast_to(symbol* p) {
if (p-isT())
return static_castT*(p);
return 0;
}
cast:
templatetypename T T symbol:as(symbol* p) {
assert(p-isT())
return static_castT(*p);
}
My concern here was never the technical feasibility, nor the
desirability of having the facility for generic code, but the amount
of the amount of typing in non-generic contexts. That is
if (cgraph_node *q = p-try_function ())
versus
if (cgraph_node *q = p-cast_to cgraph_node * ())
I thought the latter would generate objections. Does anyone object
to the extra typing?
If not, I can make that change.
Think about a more complex class hierarchy and interface consistency.
I believe you don't want this:
tree::try_decl () { .. }
tree::try_ssa_name () { ..}
tree::try_type() {...}
tree::try_int_const () {..}
tree::try_exp () { ...}
..
Rather one (or two with the const_cast version).
template T T *tree::cast_to ()
{
if (kind_ == kind_traitsT::value)
return static_castT* (this);
return 0;
}
I also think that instead of
if (cgraph_node *q = p-cast_to cgraph_node * ())
we want
if ((q = cast_to cgraph_node * (p))
I see absolutely no good reason to make cast_to a member, given
that the language has static_cast, const_cast and stuff. cast_to
would simply be our equivalent to dynamic_cast within our OO model.
Sorry, that was a think-o. Agreed there is no point in it being a
member.
Then I'd call it *_cast instead of cast_*, so, why not gcc_cast ?
Or dyn_cast (). That way
if ((q = dyn_cast function * (p))
would be how to use it.
Which function? The point with my original proposal is that the
kind of function was derivable from context, and thus can be shorter.
Small enough, compared to
if ((q = p-try_function ()))
and a lot more familiar to folks knowing C++ (and probably doesn't
make a difference to others).
Thus, please re-use or follow existing concepts.
Both are existing concepts. What I proposed is a common technique
for avoiding the cost of dynamic_cast when down casting in a class
hierarchy. Both concepts will work. I proposed what I thought
would be most convenient to programmers. However, I will change
to the other form unless someone objects.
As for the example with tree we'd then have
if ((q = dyn_cast INTEGER_CST (p)))
if we can overload on the template parameter kind (can we?
typename vs. enum?)
There are two template parameters, one for the argument type and
one for the return type. We can overload on the argument type but
not on the return type. We can, however, (indirectly) partially
specialize on the return type. We need to do that anyway to
represent the fact that not all type conversions are legal.
However, I recommend against specializing on the enum, as it would
become somewhat obscure when the hierarchy is discriminated on more
than one enum.
or use tree_cast (no I don't want dyn_cast tree_common
all around the code).
Once we have proper class hierarchies, derived to base conversions
are implicit. In the meantime, we need something else.
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On Sat, Oct 13, 2012 at 12:44 PM, Lawrence Crowl cr...@googlers.com wrote: Thus, please re-use or follow existing concepts. Both are existing concepts. What I proposed is a common technique for avoiding the cost of dynamic_cast when down casting in a class hierarchy. Both concepts will work. I proposed what I thought would be most convenient to programmers. However, I will change to the other form unless someone objects. Let me elaborate on your point. The concept you are trying to implement is that of: (a) check whether a tree is of a certain kind; (b) if so return a pointer to the (sub-)object of that kind; otherwise a null pointer. This is a standard idiom -- at least when using C++. It can be implemented in various ways. Your earlier attempt try_xxx is one example. Another common example, built into the language is dynamic_cast. The latter requires that classes involved in this test be polymorphic (because the builtin implementation needs to consult metadata that are already present with virtual functions.) Yet, another implementation is what is currently in GCC more-or-less. I think we should name the operation based on the abstract concept we are implementing as opposed the builtin language implementation. That is why I recommend is over dyn_cast or variations of it. We should be able to understand its uses without having to know the implementation details. However, I recommend against specializing on the enum, as it would become somewhat obscure when the hierarchy is discriminated on more than one enum. 100% agreed. -- Gaby
Re: Use conditional casting with symtab_node
On Thu, Oct 11, 2012 at 10:39 PM, Xinliang David Li davi...@google.com wrote:
On Thu, Oct 11, 2012 at 1:23 PM, Lawrence Crowl cr...@googlers.com wrote:
On 10/10/12, Xinliang David Li davi...@google.com wrote:
In a different thread, I proposed the following alternative to 'try_xxx':
templatetypename T T* symbol::cast_to(symbol* p) {
if (p-isT())
return static_castT*(p);
return 0;
}
cast:
templatetypename T T symbol:as(symbol* p) {
assert(p-isT())
return static_castT(*p);
}
My concern here was never the technical feasibility, nor the
desirability of having the facility for generic code, but the amount
of the amount of typing in non-generic contexts. That is
if (cgraph_node *q = p-try_function ())
versus
if (cgraph_node *q = p-cast_to cgraph_node * ())
I thought the latter would generate objections. Does anyone object
to the extra typing?
If not, I can make that change.
Think about a more complex class hierarchy and interface consistency.
I believe you don't want this:
tree::try_decl () { .. }
tree::try_ssa_name () { ..}
tree::try_type() {...}
tree::try_int_const () {..}
tree::try_exp () { ...}
..
Rather one (or two with the const_cast version).
template T T *tree::cast_to ()
{
if (kind_ == kind_traitsT::value)
return static_castT* (this);
return 0;
}
I also think that instead of
if (cgraph_node *q = p-cast_to cgraph_node * ())
we want
if ((q = cast_to cgraph_node * (p))
I see absolutely no good reason to make cast_to a member, given
that the language has static_cast, const_cast and stuff. cast_to
would simply be our equivalent to dynamic_cast within our OO model.
Then I'd call it *_cast instead of cast_*, so, why not gcc_cast ?
Or dyn_cast (). That way
if ((q = dyn_cast function * (p))
would be how to use it. Small enough, compared to
if ((q = p-try_function ()))
and a lot more familiar to folks knowing C++ (and probably doesn't make
a difference to others).
Thus, please re-use or follow existing concepts.
As for the example with tree we'd then have
if ((q = dyn_cast INTEGER_CST (p)))
if we can overload on the template parameter kind (can we? typename vs. enum?)
or use tree_cast (no I don't want dyn_cast tree_common all around the
code).
Thanks,
Richard.
thanks,
David
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On Fri, Oct 12, 2012 at 1:22 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Thu, Oct 11, 2012 at 10:39 PM, Xinliang David Li davi...@google.com
wrote:
On Thu, Oct 11, 2012 at 1:23 PM, Lawrence Crowl cr...@googlers.com wrote:
On 10/10/12, Xinliang David Li davi...@google.com wrote:
In a different thread, I proposed the following alternative to 'try_xxx':
templatetypename T T* symbol::cast_to(symbol* p) {
if (p-isT())
return static_castT*(p);
return 0;
}
cast:
templatetypename T T symbol:as(symbol* p) {
assert(p-isT())
return static_castT(*p);
}
My concern here was never the technical feasibility, nor the
desirability of having the facility for generic code, but the amount
of the amount of typing in non-generic contexts. That is
if (cgraph_node *q = p-try_function ())
versus
if (cgraph_node *q = p-cast_to cgraph_node * ())
I thought the latter would generate objections. Does anyone object
to the extra typing?
If not, I can make that change.
Think about a more complex class hierarchy and interface consistency.
I believe you don't want this:
tree::try_decl () { .. }
tree::try_ssa_name () { ..}
tree::try_type() {...}
tree::try_int_const () {..}
tree::try_exp () { ...}
..
Rather one (or two with the const_cast version).
template T T *tree::cast_to ()
{
if (kind_ == kind_traitsT::value)
return static_castT* (this);
return 0;
}
I also think that instead of
if (cgraph_node *q = p-cast_to cgraph_node * ())
we want
if ((q = cast_to cgraph_node * (p))
I see absolutely no good reason to make cast_to a member, given
that the language has static_cast, const_cast and stuff. cast_to
would simply be our equivalent to dynamic_cast within our OO model.
Then I'd call it *_cast instead of cast_*, so, why not gcc_cast ?
Or dyn_cast (). That way
if ((q = dyn_cast function * (p))
would be how to use it. Small enough, compared to
if ((q = p-try_function ()))
and a lot more familiar to folks knowing C++ (and probably doesn't make
a difference to others).
Thus, please re-use or follow existing concepts.
Agree. dyn_cast.. for try casting, and cast.. or gcc_cast..
casting with assertion sounds good.
As for the example with tree we'd then have
if ((q = dyn_cast INTEGER_CST (p)))
if we can overload on the template parameter kind (can we? typename vs. enum?)
or use tree_cast (no I don't want dyn_cast tree_common all around the
code).
yes, that also sounds good to me.
thanks,
David
Thanks,
Richard.
thanks,
David
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On Thu, Oct 11, 2012 at 7:31 AM, Xinliang David Li davi...@google.com wrote: On Fri, Oct 5, 2012 at 1:49 AM, Richard Guenther richard.guent...@gmail.com wrote: On Thu, Oct 4, 2012 at 8:16 PM, Diego Novillo dnovi...@google.com wrote: On Thu, Oct 4, 2012 at 2:14 PM, Lawrence Crowl cr...@googlers.com wrote: So, Jan Hubicka requested and approved the current spelling. What now? I don't think we should hold this up. The names Jan requested seem reasonable enough. We seem to be running in circles here. I suppose I have your promise that we'll release with consistent names. Please allocate some work hours on your side for the renaming of cgraph_node and varpool_node for the case Honza doesn't get to it in time. I see all these patches with mixed feeling - it puts breaks on all developers because they need to learn the new interface which does not bring any immediate benefit. So I think _your_ development time would be better spent by fixing open bugs or by tackling some of the existing scalability issues in GCC (rather than quoting funny '0.001% faster with 99% confidence' stuff). Interface cleanup will help GCC in the long run assuming it is done correctly. There will be short term churns for sure. However I think it is also important to get things right in as few steps as possible with a better/more carefully thought design. I agree. There is no reason to change things just to change them. Richard. David Thanks, Richard. Diego.
Re: Use conditional casting with symtab_node
On 10/10/12, Xinliang David Li davi...@google.com wrote:
In a different thread, I proposed the following alternative to 'try_xxx':
templatetypename T T* symbol::cast_to(symbol* p) {
if (p-isT())
return static_castT*(p);
return 0;
}
cast:
templatetypename T T symbol:as(symbol* p) {
assert(p-isT())
return static_castT(*p);
}
My concern here was never the technical feasibility, nor the
desirability of having the facility for generic code, but the amount
of the amount of typing in non-generic contexts. That is
if (cgraph_node *q = p-try_function ())
versus
if (cgraph_node *q = p-cast_to cgraph_node * ())
I thought the latter would generate objections. Does anyone object
to the extra typing?
If not, I can make that change.
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On Thu, Oct 11, 2012 at 1:23 PM, Lawrence Crowl cr...@googlers.com wrote:
On 10/10/12, Xinliang David Li davi...@google.com wrote:
In a different thread, I proposed the following alternative to 'try_xxx':
templatetypename T T* symbol::cast_to(symbol* p) {
if (p-isT())
return static_castT*(p);
return 0;
}
cast:
templatetypename T T symbol:as(symbol* p) {
assert(p-isT())
return static_castT(*p);
}
My concern here was never the technical feasibility, nor the
desirability of having the facility for generic code, but the amount
of the amount of typing in non-generic contexts. That is
if (cgraph_node *q = p-try_function ())
versus
if (cgraph_node *q = p-cast_to cgraph_node * ())
I thought the latter would generate objections. Does anyone object
to the extra typing?
If not, I can make that change.
Think about a more complex class hierarchy and interface consistency.
I believe you don't want this:
tree::try_decl () { .. }
tree::try_ssa_name () { ..}
tree::try_type() {...}
tree::try_int_const () {..}
tree::try_exp () { ...}
..
Rather one (or two with the const_cast version).
template T T *tree::cast_to ()
{
if (kind_ == kind_traitsT::value)
return static_castT* (this);
return 0;
}
thanks,
David
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On Tue, Oct 9, 2012 at 8:03 PM, Lawrence Crowl cr...@googlers.com wrote: I would like some clarity. Can I commit this patch? I'm thinking, yes. I will be making the gengtype changes in time for stage 1, so further renames can continue after those patches are in. Jan, Richard? Any strong objections? Diego.
Re: Use conditional casting with symtab_node
In a different thread, I proposed the following alternative to 'try_xxx':
templatetypename T T* symbol::cast_to(symbol* p) {
if (p-isT())
return static_castT*(p);
return 0;
}
cast:
templatetypename T T symbol:as(symbol* p) {
assert(p-isT())
return static_castT(*p);
}
David
On Wed, Sep 19, 2012 at 2:17 AM, Richard Guenther
richard.guent...@gmail.com wrote:
On Wed, Sep 19, 2012 at 9:29 AM, Eric Botcazou ebotca...@adacore.com wrote:
The language syntax would bind the conditional into the intializer, as in
if (varpool_node *vnode = (node-try_variable ()
vnode-finalized))
varpool_analyze_node (vnode);
which does not type-match.
So, if you want the type saftey and performance, the cascade is really
unavoidable.
Just write:
varpool_node *vnode;
if ((vnode = node-try_variable ()) vnode-finalized)
varpool_analyze_node (vnode);
This has been the standard style for the past 2 decades and trading it for
cascading if's is really a bad idea.
Indeed. Btw, can we not provide a specialization for dynamic_cast ?
This -try_... looks awkward to me compared to the more familiar
vnode = dynamic_cast varpool_node (node)
but yeah - dynamic_cast is not a template ... (but maybe there is some
standard library piece that mimics it?).
Richard.
--
Eric Botcazou
Re: Use conditional casting with symtab_node
On Wed, Sep 19, 2012 at 11:39 AM, Lawrence Crowl cr...@googlers.com wrote:
On 9/19/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Sep 19, 2012 Richard Guenther richard.guent...@gmail.com wrote:
Indeed. Btw, can we not provide a specialization for
dynamic_cast ? This -try_... looks awkward to me compared
to the more familiar
vnode = dynamic_cast varpool_node (node)
but yeah - dynamic_cast is not a template ... (but maybe there
is some standard library piece that mimics it?).
No, it is a language primitive.
but we can define out own operation with similar syntax that allows
for specialization, whose generic implementation uses dynamic_cast.
templatetypename T, typename U
T* is(U* u) {
return dynamic_castT*(u);
}
At this point, dynamic_cast is not available because we do not
yet have polymorphic types. There has been some resistance to
that notion.
Absent dynamic cast, we need to specialize for various type
combinations.
The generic implementation can simply assert.
templatetypename T bool symbol::is()
{
assert (0);
return true;
}
template bool symbol::isfunction()
{
if (type_ == FUNCTION)
return true;
return false;
}
template bool symbol::isvariable()
{
if (type_ == FUNCTION)
return false;
return true;
}
David
Function template specialization would be handy,
but C++ does not directly support that. We could work around
that. However, in the end, the fact that try_whatever is a member
function means that we can use a notation that depends on context
and so can be shorter. That is, we can write 'function' instead of
'cgraph_node *'.
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On Fri, Oct 5, 2012 at 1:49 AM, Richard Guenther richard.guent...@gmail.com wrote: On Thu, Oct 4, 2012 at 8:16 PM, Diego Novillo dnovi...@google.com wrote: On Thu, Oct 4, 2012 at 2:14 PM, Lawrence Crowl cr...@googlers.com wrote: So, Jan Hubicka requested and approved the current spelling. What now? I don't think we should hold this up. The names Jan requested seem reasonable enough. We seem to be running in circles here. I suppose I have your promise that we'll release with consistent names. Please allocate some work hours on your side for the renaming of cgraph_node and varpool_node for the case Honza doesn't get to it in time. I see all these patches with mixed feeling - it puts breaks on all developers because they need to learn the new interface which does not bring any immediate benefit. So I think _your_ development time would be better spent by fixing open bugs or by tackling some of the existing scalability issues in GCC (rather than quoting funny '0.001% faster with 99% confidence' stuff). Interface cleanup will help GCC in the long run assuming it is done correctly. There will be short term churns for sure. However I think it is also important to get things right in as few steps as possible with a better/more carefully thought design. David Thanks, Richard. Diego.
Re: Use conditional casting with symtab_node
On 10/5/12, Jan Hubicka hubi...@ucw.cz wrote:
On Thu, Oct 4, 2012 at 8:16 PM, Diego Novillo dnovi...@google.com wrote:
On Thu, Oct 4, 2012 at 2:14 PM, Lawrence Crowl cr...@googlers.com wrote:
So, Jan Hubicka requested and approved the current spelling.
What now?
I don't think we should hold this up. The names Jan requested seem
reasonable enough. We seem to be running in circles here.
I suppose I have your promise that we'll release with consistent names.
Please allocate some work hours on your side for the renaming of
cgraph_node and varpool_node for the case Honza doesn't get to it in
time.
Not that I would not agree with Richard with most of his points. To be
however
fair here, I only asked to continue naming I already introduced in:
/* Return true when NODE is function. */
static inline bool
symtab_function_p (symtab_node node)
{
return node-symbol.type == SYMTAB_FUNCTION;
}
/* Return true when NODE is variable. */
static inline bool
symtab_variable_p (symtab_node node)
{
return node-symbol.type == SYMTAB_VARIABLE;
}
Even if variable are represented by varpool and functions by cgraph, I do
not
see these names more confusing compared to
symtab_cgraph_p/symtab_varpool_p.
The most common use is when you walk the symbol table and you want to
handle
functions and variables specially.
The new interface with try_ scheme gives a bit more inconsistent feeling,
but it is just an surface, nothing really changes.
I am not happy with current naming scheme and also with the fact that for
gengtype reasons we also have symtab_node typedef, but for varpool and
cgraph
we use struct (this is because symtab_node has to be union without GTY
supporting inheritance).
Introducing symtab I did not have much other options and I expected that
at the end of this stage1 I will clean it up. This is, well, more or less
now
when assuming that there are no major patches to this area just to appear
for this stage1.
I guess we all agreed we want to drop cgraph/varpool nodes in favor of
functions/ variables. How realistic is for gengtype to support inheritance
this release cycle? I would really like to see these three turned into
classes
with the inheritance this release cycle. Renaming them during the process
should be easy and just a nice bonus.
I would like some clarity. Can I commit this patch?
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On Thu, Oct 4, 2012 at 8:16 PM, Diego Novillo dnovi...@google.com wrote: On Thu, Oct 4, 2012 at 2:14 PM, Lawrence Crowl cr...@googlers.com wrote: So, Jan Hubicka requested and approved the current spelling. What now? I don't think we should hold this up. The names Jan requested seem reasonable enough. We seem to be running in circles here. I suppose I have your promise that we'll release with consistent names. Please allocate some work hours on your side for the renaming of cgraph_node and varpool_node for the case Honza doesn't get to it in time. I see all these patches with mixed feeling - it puts breaks on all developers because they need to learn the new interface which does not bring any immediate benefit. So I think _your_ development time would be better spent by fixing open bugs or by tackling some of the existing scalability issues in GCC (rather than quoting funny '0.001% faster with 99% confidence' stuff). Thanks, Richard. Diego.
Re: Use conditional casting with symtab_node
On Thu, Oct 4, 2012 at 8:16 PM, Diego Novillo dnovi...@google.com wrote:
On Thu, Oct 4, 2012 at 2:14 PM, Lawrence Crowl cr...@googlers.com wrote:
So, Jan Hubicka requested and approved the current spelling.
What now?
I don't think we should hold this up. The names Jan requested seem
reasonable enough. We seem to be running in circles here.
I suppose I have your promise that we'll release with consistent names.
Please allocate some work hours on your side for the renaming of
cgraph_node and varpool_node for the case Honza doesn't get to it in time.
Not that I would not agree with Richard with most of his points. To be however
fair here, I only asked to continue naming I already introduced in:
/* Return true when NODE is function. */
static inline bool
symtab_function_p (symtab_node node)
{
return node-symbol.type == SYMTAB_FUNCTION;
}
/* Return true when NODE is variable. */
static inline bool
symtab_variable_p (symtab_node node)
{
return node-symbol.type == SYMTAB_VARIABLE;
}
Even if variable are represented by varpool and functions by cgraph, I do not
see these names more confusing compared to symtab_cgraph_p/symtab_varpool_p.
The most common use is when you walk the symbol table and you want to handle
functions and variables specially.
The new interface with try_ scheme gives a bit more inconsistent feeling,
but it is just an surface, nothing really changes.
I am not happy with current naming scheme and also with the fact that for
gengtype reasons we also have symtab_node typedef, but for varpool and cgraph
we use struct (this is because symtab_node has to be union without GTY
supporting inheritance).
Introducing symtab I did not have much other options and I expected that
at the end of this stage1 I will clean it up. This is, well, more or less now
when assuming that there are no major patches to this area just to appear
for this stage1.
I guess we all agreed we want to drop cgraph/varpool nodes in favor of
functions/ variables. How realistic is for gengtype to support inheritance
this release cycle? I would really like to see these three turned into classes
with the inheritance this release cycle. Renaming them during the process
should be easy and just a nice bonus.
Honza
Re: Use conditional casting with symtab_node
- Original Message - I see all these patches with mixed feeling - it puts breaks on all developers because they need to learn the new interface which does not bring any immediate benefit. So I think _your_ development time would be better spent by fixing open bugs or by tackling some of the existing scalability issues in GCC (rather than quoting funny '0.001% faster with 99% confidence' stuff). This tone is unnecessary. I, for one, think that it's excellent that Lawrence is writing these cleanup patches and measuring what impact they have on performance. Bonus points that they are making the compiler faster. Speed of the compiler *is* a scalability issue, and it's one that GCC doesn't appear to have paid all that much attention to over the years. -Nathan
Re: Use conditional casting with symtab_node
On Fri, Oct 5, 2012 at 12:50 PM, Nathan Froyd froy...@mozilla.com wrote: - Original Message - I see all these patches with mixed feeling - it puts breaks on all developers because they need to learn the new interface which does not bring any immediate benefit. So I think _your_ development time would be better spent by fixing open bugs or by tackling some of the existing scalability issues in GCC (rather than quoting funny '0.001% faster with 99% confidence' stuff). This tone is unnecessary. Sorry, that wasn't intended. I question these numbers because unless you bootstrap say 100 times the noise in bootstrap speed is way too high to make such claims. Of course critical information is missing: The new code bootstraps .616% faster with a 99% confidence of being faster. 99% confidence on what basis? What's your sample size? Why does the patch need this kind of marketing? I, for one, think that it's excellent that Lawrence is writing these cleanup patches and measuring what impact they have on performance. Bonus points that they are making the compiler faster. Speed of the compiler *is* a scalability issue, and it's one that GCC doesn't appear to have paid all that much attention to over the years. I just don't believe the 0.5% numbers. Richard. -Nathan
Re: Use conditional casting with symtab_node
On Fri, Oct 5, 2012 at 7:05 AM, Richard Guenther richard.guent...@gmail.com wrote: Sorry, that wasn't intended. I question these numbers because unless you bootstrap say 100 times the noise in bootstrap speed is way too high to make such claims. Of course critical information is missing: I agree with Nathan. Your tone is sometimes borderline insulting. It creates unnecessary friction and does not serve anybody's purpose. There is no need to be so antagonistic at all times. The new code bootstraps .616% faster with a 99% confidence of being faster. 99% confidence on what basis? What's your sample size? Perhaps Lawrence can explain a bit more how he's getting these numbers. But they are not pulled out of thin air and he does go to the extra effort of measuring them and computing the differences. Why does the patch need this kind of marketing? Because (a) we have always said that we want to make sure that the C++ conversion provides useful benefits, and (b) there has been so much negative pressure on our work, that we sometimes try to find some benefit when reality may provide neutral results. I, for one, think that it's excellent that Lawrence is writing these cleanup patches and measuring what impact they have on performance. Bonus points that they are making the compiler faster. Speed of the compiler *is* a scalability issue, and it's one that GCC doesn't appear to have paid all that much attention to over the years. I just don't believe the 0.5% numbers. Then ask. Don't mock, please. Diego.
Re: Use conditional casting with symtab_node
On Fri, Oct 5, 2012 at 2:43 PM, Diego Novillo wrote: Because (...) there has been so much negative pressure on our work, that we sometimes try to find some benefit when reality may provide neutral results. When people say your work sucks, they probably don't mean to apply negative pressure but just that there is a strong pulling force in your direction ;-) Ciao! Steven
Re: Use conditional casting with symtab_node
On Fri, Oct 5, 2012 at 2:43 PM, Diego Novillo wrote: Because (...) there has been so much negative pressure on our work, that we sometimes try to find some benefit when reality may provide neutral results. When people say your work sucks, they probably don't mean to apply negative pressure but just that there is a strong pulling force in your direction ;-) Ciao! Steven
Re: Use conditional casting with symtab_node
On 10/5/12, Diego Novillo dnovi...@google.com wrote: On Oct 5, 2012 Richard Guenther richard.guent...@gmail.com wrote: Sorry, that wasn't intended. I question these numbers because unless you bootstrap say 100 times the noise in bootstrap speed is way too high to make such claims. Of course critical information is missing: I agree with Nathan. Your tone is sometimes borderline insulting. It creates unnecessary friction and does not serve anybody's purpose. There is no need to be so antagonistic at all times. The new code bootstraps .616% faster with a 99% confidence of being faster. 99% confidence on what basis? What's your sample size? Perhaps Lawrence can explain a bit more how he's getting these numbers. But they are not pulled out of thin air and he does go to the extra effort of measuring them and computing the differences. The intent of the work is to compare the performance of the unmodified compiler and the compiler with my patch. For each compiler, I run the third stage of the boot with -ftime-report ten times. By running the third stage, I test two things. Stage two has all the benefits of any performance improvements that the restructuring can deliver to end users. But since it is compiling stage three, it is also accounting for any increased time that the new C++ code takes in the bootstrap. The end customer won't pay that cost, but it was a concern among GCC developers. By parsing the log files, I extract total CPU time for each run. So, I have two samples, each with ten data points. Each sample has a sampled mean and a variance, from which you can compute a confidence interval, in which the true mean is likely to be. You can then compare the two confidence intervals to determine the likely hood that one is better or worse than the other. So, in the statement The new code bootstraps .616% faster with a 99% confidence of being faster, the last phrase says if we were to run that same experiment 100 times, we might get one case where the compiler was slower. For most purposes, a 95% confidence is sufficient for medical interventions. Compile-time isn't that important, so we could easily get by on 70% confidence. In any event, the sample size is only relevant to the extent that larger sample sizes yield more confidence. More consistent runs also yield more confidence. Algorithmic changes, which would yield larger difference, would also yield more confidence. Since I report the confidence, you don't need to worry about sample size or isolation from system contention, etc. All those issues would have affected confidence. Why does the patch need this kind of marketing? Because (a) we have always said that we want to make sure that the C++ conversion provides useful benefits, and (b) there has been so much negative pressure on our work, that we sometimes try to find some benefit when reality may provide neutral results. Yes, in particular, there was some concern that the cost of compiling the templates used in the hash tables would increase the bootstrap time significantly. In these cases, I have shown that the benefit of using them exceeds the cost of compiling them. If no one cares about these time reports, then I will gladly stop spending the effort to make them. I, for one, think that it's excellent that Lawrence is writing these cleanup patches and measuring what impact they have on performance. Bonus points that they are making the compiler faster. Speed of the compiler *is* a scalability issue, and it's one that GCC doesn't appear to have paid all that much attention to over the years. I just don't believe the 0.5% numbers. Then ask. Don't mock, please. -- Lawrence Crowl
Re: Use conditional casting with symtab_node
On Fri, Oct 5, 2012 at 11:50 PM, Lawrence Crowl cr...@googlers.com wrote: If no one cares about these time reports, then I will gladly stop spending the effort to make them. It's not that no-one cases, I think, but the mathematics don't have to be so complicated. Just showing or saying there's no significant compile time impact is quite enough. As for the confidence level, it depends so much on what else your machine is doing (other users, kernel, whatever) that statistics become just that: statistics, those worse-than-damned-lies numbers :-) FWIW: I usually do 3 runs on a set of pre-processed cc1-i files (excluding very small files, but also including a few non-cc1 files e.g. interpret.cc from libjava) with patched and unpatched compiler, and declare victory if there's no difference greater than a few % on any of the test files. Ciao! Steven
Re: Use conditional casting with symtab_node
On Thu, Oct 4, 2012 at 2:14 PM, Lawrence Crowl cr...@googlers.com wrote: So, Jan Hubicka requested and approved the current spelling. What now? I don't think we should hold this up. The names Jan requested seem reasonable enough. We seem to be running in circles here. Diego.
Re: Use conditional casting with symtab_node
Updated Patch
Add functions symtab_node_def::try_function and symtab_node_def::try_variable.
These function return a pointer to the more specific type (e.g. cgraph_node*)
if and only if the general type (symtab_node aka symtab_node_def*) is an
instance of the specific type. These functions are essentially checked down
casts.
These functions reduce compile time and increase type safety when treating a
generic item as a more specific item. In essence, the code change is from
if (symtab_function_p (node))
{
struct cgraph_node *cnode = cgraph (node);
}
to
if (cgraph_node *cnode = node-try_function ())
{
}
The necessary conditional test defines a variable that holds a known good
pointer to the specific item and avoids subsequent conversion calls and
the assertion checks that may come with them.
When, the property test is embedded within a larger condition, the variable
declaration gets pulled out of the condition. (This leaves some room for
using the variable inappropriately.)
if (symtab_variable_p (node)
varpool (node)-finalized)
varpool_analyze_node (varpool (node));
becomes
varpool_node *vnode = node-try_variable ();
if (vnode vnode-finalized)
varpool_analyze_node (vnode);
Note that we have converted two sets of assertions in the calls to varpool
into safe and efficient use of a variable.
There are remaining calls to symtab_function_p and symtab_variable_p that
do not involve a pointer to a more specific type. These have been converted
to calls to a member functions symtab_node_def::is_function and
symtab_node_def::is_variable. The original predicate functions have been
removed.
The cgraph.h header defined both a struct and a function with the name
varpool_node. This name overloading can cause some unintuitive error messages
when, as is common in C++, one omits the struct keyword when using the type.
I have renamed the function to varpool_node_for_decl.
The new code bootstraps .616% faster with a 99% confidence of being faster.
Tested on x86_64.
Okay for trunk?
Index: gcc/ChangeLog
2012-10-02 Lawrence Crowl cr...@google.com
* cgraph.h (varpool_node): Rename to varpool_node_for_decl.
Adjust callers to match.
(symtab_node_def::try_function): New.
Change most calls to symtab_function_p with calls to
symtab_node_def::try_function.
(symtab_node_def::try_variable): New.
Change most calls to symtab_variable_p with calls to
symtab_node_def::try_variable.
(symtab_function_p): Rename to symtab_node_def::is_function.
Adjust remaining callers to match.
(symtab_variable_p): Rename to symtab_node_def::is_variable.
Adjust remaining callers to match.
* cgraph.c (cgraph_node_for_asm): Remove redundant call to
symtab_node_for_asm.
* cgraphunit.c (symbol_finalized_and_needed): New.
(symbol_finalized): New.
(cgraph_analyze_functions): Split complicated conditionals out into
above new functions.
Index: gcc/lto-symtab.c
===
--- gcc/lto-symtab.c(revision 192010)
+++ gcc/lto-symtab.c(working copy)
@@ -566,11 +566,11 @@ lto_symtab_merge_cgraph_nodes_1 (symtab_
if (!symtab_real_symbol_p (e))
continue;
- if (symtab_function_p (e)
- !DECL_BUILT_IN (e-symbol.decl))
- lto_cgraph_replace_node (cgraph (e), cgraph (prevailing));
- if (symtab_variable_p (e))
- lto_varpool_replace_node (varpool (e), varpool (prevailing));
+ cgraph_node *ce = e-try_function ();
+ if (ce !DECL_BUILT_IN (e-symbol.decl))
+ lto_cgraph_replace_node (ce, cgraph (prevailing));
+ if (varpool_node *ve = e-try_variable ())
+ lto_varpool_replace_node (ve, varpool (prevailing));
}
return;
Index: gcc/cgraphbuild.c
===
--- gcc/cgraphbuild.c (revision 192010)
+++ gcc/cgraphbuild.c (working copy)
@@ -84,7 +84,7 @@ record_reference (tree *tp, int *walk_su
if (TREE_CODE (decl) == VAR_DECL)
{
- struct varpool_node *vnode = varpool_node (decl);
+ struct varpool_node *vnode = varpool_node_for_decl (decl);
ipa_record_reference ((symtab_node)ctx-varpool_node,
(symtab_node)vnode,
IPA_REF_ADDR, NULL);
@@ -123,7 +123,7 @@ record_type_list (struct cgraph_node *no
type = TREE_OPERAND (type, 0);
if (TREE_CODE (type) == VAR_DECL)
{
- struct varpool_node *vnode = varpool_node (type);
+ struct varpool_node *vnode = varpool_node_for_decl (type);
ipa_record_reference ((symtab_node)node,
(symtab_node)vnode,
IPA_REF_ADDR, NULL);
@@ -233,7 +233,7 @@ mark_address (gimple stmt, tree addr,
Re: Use conditional casting with symtab_node
Add functions symtab_node_def::try_function and symtab_node_def::try_variable.
These function return a pointer to the more specific type (e.g. cgraph_node*)
if and only if the general type (symtab_node aka symtab_node_def*) is an
instance of the specific type. These functions are essentially checked down
casts.
These functions reduce compile time and increase type safety when treating a
generic item as a more specific item. In essence, the code change is from
if (symtab_function_p (node))
{
struct cgraph_node *cnode = cgraph (node);
}
to
if (cgraph_node *cnode = node-try_function ())
{
}
The necessary conditional test defines a variable that holds a known good
pointer to the specific item and avoids subsequent conversion calls and
the assertion checks that may come with them.
When, the property test is embedded within a larger condition, the variable
declaration gets pulled out of the condition. (This leaves some room for
using the variable inappropritately.)
if (symtab_variable_p (node)
varpool (node)-finalized)
varpool_analyze_node (varpool (node));
becomes
varpool_node *vnode = node-try_variable ();
if (vnode vnode-finalized)
varpool_analyze_node (vnode);
Note that we have converted two sets of assertions in the calls to varpool
into safe and efficient use of a variable.
There are remaining calls to symtab_function_p and symtab_variable_p that
do not involve a pointer to a more specific type. These have been converted
to calls to a member functions symtab_node_def::is_function and
symtab_node_def::is_variable. The original predicate functions have been
removed.
The cgraph.h header defined both a struct and a function with the name
varpool_node. This name overloading can cause some unintuitive error messages
when, as is common in C++, one omits the struct keyword when using the type.
I have renamed the function to varpool_node_for_tree.
The new code bootstraps .616% faster with a 99% confidence of being faster.
Tested on x86_64.
Okay for trunk?
Index: gcc/ChangeLog
2012-09-18 Lawrence Crowl cr...@google.com
* cgraph.h (varpool_node): Rename to varpool_node_for_tree.
Adjust callers to match.
(symtab_node_def::try_function): New.
Change most calls to symtab_function_p with calls to
symtab_node_def::try_function.
(symtab_node_def::try_variable): New.
Change most calls to symtab_variable_p with calls to
symtab_node_def::try_variable.
(symtab_function_p): Rename to symtab_node_def::is_function.
Adjust remaining callers to match.
(symtab_variable_p): Rename to symtab_node_def::is_variable.
Adjust remaining callers to match.
* cgraph.c (cgraph_node_for_asm): Remove redundant call to
symtab_node_for_asm.
* graphunit.c (symbol_finalized_and_needed): New.
(symbol_finalized): New.
(cgraph_analyze_functions): Split complicated conditionals out into
above new functions.
Index: gcc/lto-symtab.c
===
--- gcc/lto-symtab.c(revision 191403)
+++ gcc/lto-symtab.c(working copy)
@@ -743,7 +743,7 @@ lto_symtab_merge_cgraph_nodes_1 (void **
{
if (!prevailing-vnode)
{
- prevailing-vnode = varpool_node (prevailing-decl);
+ prevailing-vnode = varpool_node_for_tree (prevailing-decl);
prevailing-vnode-alias = true;
}
lto_varpool_replace_node (e-vnode, prevailing-vnode);
Index: gcc/cgraphbuild.c
===
--- gcc/cgraphbuild.c (revision 191403)
+++ gcc/cgraphbuild.c (working copy)
@@ -84,7 +84,7 @@ record_reference (tree *tp, int *walk_su
if (TREE_CODE (decl) == VAR_DECL)
{
- struct varpool_node *vnode = varpool_node (decl);
+ struct varpool_node *vnode = varpool_node_for_tree (decl);
ipa_record_reference ((symtab_node)ctx-varpool_node,
(symtab_node)vnode,
IPA_REF_ADDR, NULL);
@@ -123,7 +123,7 @@ record_type_list (struct cgraph_node *no
type = TREE_OPERAND (type, 0);
if (TREE_CODE (type) == VAR_DECL)
{
- struct varpool_node *vnode = varpool_node (type);
+ struct varpool_node *vnode = varpool_node_for_tree (type);
ipa_record_reference ((symtab_node)node,
(symtab_node)vnode,
IPA_REF_ADDR, NULL);
@@ -233,7 +233,7 @@ mark_address (gimple stmt, tree addr, vo
else if (addr TREE_CODE (addr) == VAR_DECL
(TREE_STATIC (addr) || DECL_EXTERNAL (addr)))
{
- struct varpool_node *vnode = varpool_node (addr);
+ struct varpool_node *vnode = varpool_node_for_tree (addr);
ipa_record_reference
Re: Use conditional casting with symtab_node
Hi, On Wed, 19 Sep 2012, Richard Guenther wrote: Just write: varpool_node *vnode; if ((vnode = node-try_variable ()) vnode-finalized) varpool_analyze_node (vnode); This has been the standard style for the past 2 decades and trading it for cascading if's is really a bad idea. Indeed. Btw, can we not provide a specialization for dynamic_cast ? This -try_... looks awkward to me compared to the more familiar vnode = dynamic_cast varpool_node (node) Gah. The less '' characters in source code the better. I'm not thrilled, but prefer the -try_ thingy. And yes, cascading if's for this idiom is bad. Ciao, Michael.
Re: Use conditional casting with symtab_node
Hi,
On Wed, 19 Sep 2012, Lawrence Crowl wrote:
On 9/19/12, Eric Botcazou ebotca...@adacore.com wrote:
The language syntax would bind the conditional into the
intializer, as in
if (varpool_node *vnode = (node-try_variable ()
vnode-finalized))
varpool_analyze_node (vnode);
which does not type-match.
So, if you want the type saftey and performance, the cascade
is really unavoidable.
Just write:
varpool_node *vnode;
if ((vnode = node-try_variable ()) vnode-finalized)
varpool_analyze_node (vnode);
This has been the standard style for the past 2 decades and
trading it for cascading if's is really a bad idea.
Assignments in if statements are known to cause confusion.
So? Make it:
varpool_node *vnode = node-try_variable ();
if (vnode vnode-finalized)
varpool_analyze_node (vnode);
The point of the change is to limit the scope of the variable
to the if statement, which prevents its unintended use later.
I'm not worried about this.
Why do you think cascading ifs is a bad idea?
Precedent. Confusion in case of dangling else (requiring more {}, and
hence even more indentation). Ugly.
Ciao,
Michael.
Re: Use conditional casting with symtab_node
On 9/20/12, Michael Matz m...@suse.de wrote:
On Wed, 19 Sep 2012, Lawrence Crowl wrote:
On 9/19/12, Eric Botcazou ebotca...@adacore.com wrote:
The language syntax would bind the conditional into the
intializer, as in
if (varpool_node *vnode = (node-try_variable ()
vnode-finalized))
varpool_analyze_node (vnode);
which does not type-match.
So, if you want the type saftey and performance, the cascade
is really unavoidable.
Just write:
varpool_node *vnode;
if ((vnode = node-try_variable ()) vnode-finalized)
varpool_analyze_node (vnode);
This has been the standard style for the past 2 decades and
trading it for cascading if's is really a bad idea.
Assignments in if statements are known to cause confusion.
So? Make it:
varpool_node *vnode = node-try_variable ();
if (vnode vnode-finalized)
varpool_analyze_node (vnode);
The point of the change is to limit the scope of the variable
to the if statement, which prevents its unintended use later.
I'm not worried about this.
It is helpful to have the language and the usage in concurrence.
Okay, so unless someone objects, I'll move the variable out when
it introduces a cacade.
Why do you think cascading ifs is a bad idea?
Precedent. Confusion in case of dangling else (requiring more {},
and hence even more indentation). Ugly.
I generally take ugly as an indication that the function needs
refactoring.
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On Thu, Sep 20, 2012 at 1:53 PM, Lawrence Crowl cr...@googlers.com wrote:
Why do you think cascading ifs is a bad idea?
Precedent. Confusion in case of dangling else (requiring more {},
and hence even more indentation). Ugly.
I generally take ugly as an indication that the function needs
refactoring.
Hear! Hear! Hear!
Sadly, many functions in GCC (at least in cp/) are in that category :-(
-- Gaby
Re: Use conditional casting with symtab_node
On 9/20/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Sep 20, 2012 Lawrence Crowl cr...@googlers.com wrote:
Why do you think cascading ifs is a bad idea?
Precedent. Confusion in case of dangling else (requiring
more {}, and hence even more indentation). Ugly.
I generally take ugly as an indication that the function needs
refactoring.
Hear! Hear! Hear!
Sadly, many functions in GCC (at least in cp/) are in that
category :-(
It is not just GCC. Most large codebases I've seen have had lots of
large functions. It takes persistent effort to beat back entropy.
I once chatted with a guy who worked on a project with a strict
requirement that every function fit on one screen, and that was
when screens were 24 lines. He said their bug rate was really low.
And the one function they made an exception for turned out the have
the worst bug in it.
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
The language syntax would bind the conditional into the intializer, as in if (varpool_node *vnode = (node-try_variable () vnode-finalized)) varpool_analyze_node (vnode); which does not type-match. So, if you want the type saftey and performance, the cascade is really unavoidable. Just write: varpool_node *vnode; if ((vnode = node-try_variable ()) vnode-finalized) varpool_analyze_node (vnode); This has been the standard style for the past 2 decades and trading it for cascading if's is really a bad idea. -- Eric Botcazou
Re: Use conditional casting with symtab_node
On Wed, Sep 19, 2012 at 9:29 AM, Eric Botcazou ebotca...@adacore.com wrote: The language syntax would bind the conditional into the intializer, as in if (varpool_node *vnode = (node-try_variable () vnode-finalized)) varpool_analyze_node (vnode); which does not type-match. So, if you want the type saftey and performance, the cascade is really unavoidable. Just write: varpool_node *vnode; if ((vnode = node-try_variable ()) vnode-finalized) varpool_analyze_node (vnode); This has been the standard style for the past 2 decades and trading it for cascading if's is really a bad idea. Indeed. Btw, can we not provide a specialization for dynamic_cast ? This -try_... looks awkward to me compared to the more familiar vnode = dynamic_cast varpool_node (node) but yeah - dynamic_cast is not a template ... (but maybe there is some standard library piece that mimics it?). Richard. -- Eric Botcazou
Re: Use conditional casting with symtab_node
On Wed, Sep 19, 2012 at 4:17 AM, Richard Guenther
richard.guent...@gmail.com wrote:
On Wed, Sep 19, 2012 at 9:29 AM, Eric Botcazou ebotca...@adacore.com wrote:
The language syntax would bind the conditional into the intializer, as in
if (varpool_node *vnode = (node-try_variable ()
vnode-finalized))
varpool_analyze_node (vnode);
which does not type-match.
So, if you want the type saftey and performance, the cascade is really
unavoidable.
Just write:
varpool_node *vnode;
if ((vnode = node-try_variable ()) vnode-finalized)
varpool_analyze_node (vnode);
This has been the standard style for the past 2 decades and trading it for
cascading if's is really a bad idea.
Indeed. Btw, can we not provide a specialization for dynamic_cast ?
No, it is a language primitive.
but we can define out own operation with similar syntax that allows
for specialization, whose generic implementation uses dynamic_cast.
templatetypename T, typename U
T* is(U* u) {
return dynamic_castT*(u);
}
This -try_... looks awkward to me compared to the more familiar
vnode = dynamic_cast varpool_node (node)
but yeah - dynamic_cast is not a template ... (but maybe there is some
standard library piece that mimics it?).
Richard.
--
Eric Botcazou
Re: Use conditional casting with symtab_node
On 9/19/12, Eric Botcazou ebotca...@adacore.com wrote: The language syntax would bind the conditional into the intializer, as in if (varpool_node *vnode = (node-try_variable () vnode-finalized)) varpool_analyze_node (vnode); which does not type-match. So, if you want the type saftey and performance, the cascade is really unavoidable. Just write: varpool_node *vnode; if ((vnode = node-try_variable ()) vnode-finalized) varpool_analyze_node (vnode); This has been the standard style for the past 2 decades and trading it for cascading if's is really a bad idea. Assignments in if statements are known to cause confusion. The point of the change is to limit the scope of the variable to the if statement, which prevents its unintended use later. It acts like a type switch. Why do you think cascading ifs is a bad idea? -- Lawrence Crowl
Re: Use conditional casting with symtab_node
On 9/19/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Sep 19, 2012 Richard Guenther richard.guent...@gmail.com wrote:
Indeed. Btw, can we not provide a specialization for
dynamic_cast ? This -try_... looks awkward to me compared
to the more familiar
vnode = dynamic_cast varpool_node (node)
but yeah - dynamic_cast is not a template ... (but maybe there
is some standard library piece that mimics it?).
No, it is a language primitive.
but we can define out own operation with similar syntax that allows
for specialization, whose generic implementation uses dynamic_cast.
templatetypename T, typename U
T* is(U* u) {
return dynamic_castT*(u);
}
At this point, dynamic_cast is not available because we do not
yet have polymorphic types. There has been some resistance to
that notion.
Absent dynamic cast, we need to specialize for various type
combinations. Function template specialization would be handy,
but C++ does not directly support that. We could work around
that. However, in the end, the fact that try_whatever is a member
function means that we can use a notation that depends on context
and so can be shorter. That is, we can write 'function' instead of
'cgraph_node *'.
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On Wed, Sep 19, 2012 at 1:39 PM, Lawrence Crowl cr...@googlers.com wrote:
On 9/19/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Sep 19, 2012 Richard Guenther richard.guent...@gmail.com wrote:
Indeed. Btw, can we not provide a specialization for
dynamic_cast ? This -try_... looks awkward to me compared
to the more familiar
vnode = dynamic_cast varpool_node (node)
but yeah - dynamic_cast is not a template ... (but maybe there
is some standard library piece that mimics it?).
No, it is a language primitive.
but we can define out own operation with similar syntax that allows
for specialization, whose generic implementation uses dynamic_cast.
templatetypename T, typename U
T* is(U* u) {
return dynamic_castT*(u);
}
At this point, dynamic_cast is not available because we do not
yet have polymorphic types. There has been some resistance to
that notion.
Hmm, when did we rule that out?
We currently implement dynamic_cast using the poor man's simulation
based on tree_code checking. We can just as well give such
simulation the is notation.
Absent dynamic cast, we need to specialize for various type
combinations. Function template specialization would be handy,
but C++ does not directly support that. We could work around
that.
We can always use the standard workaround: call a static member
function of a class template that can be specialized at will.
However, in the end, the fact that try_whatever is a member
function means that we can use a notation that depends on context
and so can be shorter. That is, we can write 'function' instead of
'cgraph_node *'.
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
On 9/19/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Wed, Sep 19, 2012 at 1:39 PM, Lawrence Crowl cr...@googlers.com wrote:
On 9/19/12, Gabriel Dos Reis g...@integrable-solutions.net wrote:
On Sep 19, 2012 Richard Guenther richard.guent...@gmail.com wrote:
Indeed. Btw, can we not provide a specialization for
dynamic_cast ? This -try_... looks awkward to me compared
to the more familiar
vnode = dynamic_cast varpool_node (node)
but yeah - dynamic_cast is not a template ... (but maybe there
is some standard library piece that mimics it?).
No, it is a language primitive.
but we can define out own operation with similar syntax that allows
for specialization, whose generic implementation uses dynamic_cast.
templatetypename T, typename U
T* is(U* u) {
return dynamic_castT*(u);
}
At this point, dynamic_cast is not available because we do not
yet have polymorphic types. There has been some resistance to
that notion.
Hmm, when did we rule that out?
We have not ruled it out, but folks are, rightly, concerned about any
size increase in critical data structures. We are also currently
lacking a gengtype that will handle inheritance. So, for now at
least, we need a scheme that will work across both inheritance and
our current tag/union approach.
We currently implement dynamic_cast using the poor man's simulation
based on tree_code checking. We can just as well give such
simulation the is notation.
Absent dynamic cast, we need to specialize for various type
combinations. Function template specialization would be handy,
but C++ does not directly support that. We could work around
that.
We can always use the standard workaround: call a static member
function of a class template that can be specialized at will.
However, in the end, the fact that try_whatever is a member
function means that we can use a notation that depends on context
and so can be shorter. That is, we can write 'function' instead of
'cgraph_node *'.
--
Lawrence Crowl
Re: Use conditional casting with symtab_node
When, the property test is embedded within a larger condition, a little restructuring is required to pull out the secondary conditions. For example, if (symtab_variable_p (node) varpool (node)-finalized) varpool_analyze_node (varpool (node)); becomes if (varpool_node *vnode = node-try_variable ()) if (vnode-finalized) varpool_analyze_node (vnode); Please avoid cascading if's like this, use the existing idiom instead. -- Eric Botcazou
Re: Use conditional casting with symtab_node
On 9/18/12, Eric Botcazou ebotca...@adacore.com wrote: When, the property test is embedded within a larger condition, a little restructuring is required to pull out the secondary conditions. For example, if (symtab_variable_p (node) varpool (node)-finalized) varpool_analyze_node (varpool (node)); becomes if (varpool_node *vnode = node-try_variable ()) if (vnode-finalized) varpool_analyze_node (vnode); Please avoid cascading if's like this, use the existing idiom instead. The language syntax would bind the conditional into the intializer, as in if (varpool_node *vnode = (node-try_variable () vnode-finalized)) varpool_analyze_node (vnode); which does not type-match. So, if you want the type saftey and performance, the cascade is really unavoidable. -- Lawrence Crowl
Re: Use conditional casting with symtab_node
On 9/18/12, Richard Guenther richard.guent...@gmail.com wrote: On Sep 18, 2012 Lawrence Crowl cr...@googlers.com wrote: * cgraph.h (varpool_node): Rename to varpool_node_for_tree. Sure it should be varpool_node_for_decl, if any. Or varpool_node_from_decl (grep for what is more common) Grep says _for_decl wins over _from_decl by 113 to 74. I will make that change unless I hear an objection soon. -- Lawrence Crowl
