[PATCH][RFC] Introduce type safety and debuggability into tree.h

[PATCH][RFC] Introduce type safety and debuggability into tree.h

[Simon Baldwin]

[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]

This patch replaces some of the function-like macros in gcc/tree.h with 
static inlined function equivalents.

The aim is to improve the experience of debugging gcc with gdb, and to 
introduce some type-safety into tree accessor functions.  There are 
around 450 such macros in type.h.  This patch addresses the first couple 
of dozen as an initial foray into refactoring.  Lengthy, but 
functionally neutral.

Confirmed with full bootstrap across all languages, and with the full 
testsuite across those same languages.  Compiler throughput for 
optimized gcc builds is the same as before the patch, within the limits 
of timing stability.

Does a patch of this nature seem useful?


gcc/gcc/Changelog
2007-08-09  Simon Baldwin  <simonb@google.com>

        * tree.h: Replaced the following macros with equivalent static 
inline
        functions: CODE_CONTAINS_STRUCT, TREE_CODE_CLASS_STRING,
        TREE_CODE_CLASS, EXCEPTIONAL_CLASS_P, CONSTANT_CLASS_P, TYPE_P,
        DECL_P, MTAG_P, VAR_OR_FUNCTION_DECL_P, INDIRECT_REF_P,
        REFERENCE_CLASS_P COMPARISON_CLASS_P, UNARY_CLASS_P, BINARY_CLASS_P,
        STATEMENT_CLASS_P, VL_EXP_CLASS_P, EXPRESSION_CLASS_P,
        IS_TYPE_OR_DECL_P, IS_EXPR_CODE_CLASS, IS_GIMPLE_STMT_CODE_CLASS,
        OMP_DIRECTIVE_P, TREE_CODE_LENGTH
        * alias.c, attribs.c, builtins.c, c-common.c, c-decl.c, c-format.c,
        c-objc-common.c, c-parser.c, c-pragma.c, c-pretty-print.c,
        c-semantics.c, c-typeck.c, calls.c, cfgexpand.c, cgraphbuild.c,
        config/alpha/alpha.c, config/arm/arm.c, config/avr/avr.c,...

...remainder in compressed attachment.


[-- Attachment #2: refactor_tree.patch.bz2 --]
[-- Type: application/x-bzip, Size: 80278 bytes --]

Re: [PATCH][RFC] Introduce type safety and debuggability into tree.h

[Mark Mitchell]

Simon Baldwin wrote:
> This patch replaces some of the function-like macros in gcc/tree.h with
> static inlined function equivalents.

I like this idea.

Just to double-check performance: you've compared performance of a
bootstrapped --disable-checking compiler with this change, right?  What
about size of the compiler?

I do have one request about this patch: make it easier for yourself by
not changing the case of the macros.  That will avoid the mass changes
through all the other files, and will make life a lot easier for people
who backport or merge patches often.  (Yes, that's self-interested, but
I don't think it's just CodeSourcery that does that.)  Sure, the coding
standards may say that ALL_CAPS is for macros, but I just don't see the
value in this kind of mass change.

This isn't an approval, because I want you to confirm the performance
questions above, and because I want to give other people a chance to
object.  But, I will approve this strategy assuming that others don't
complain.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [PATCH][RFC] Introduce type safety and debuggability into tree.h

[Diego Novillo]

On 8/9/07 4:39 PM, Mark Mitchell wrote:

> This isn't an approval, because I want you to confirm the performance
> questions above, and because I want to give other people a chance to
> object.  But, I will approve this strategy assuming that others don't
> complain.

I also like this approach quite a bit.  Regarding the capitalization
change, I don't have a strong opinion either way by I'm slightly in
favour of going to the lower case version.

The merge pain will happen at one point or another, if we don't do it
now I think we'll never get it done because the temptation to maintain
status quo will be always there.

Re: [PATCH][RFC] Introduce type safety and debuggability into tree.h

[Mark Mitchell]

Diego Novillo wrote:

> The merge pain will happen at one point or another, if we don't do it
> now I think we'll never get it done because the temptation to maintain
> status quo will be always there.

Yes, and, hence, we will always avoid the merge pain. :-)

I just don't see that there's that much advantage in advertising whether
or not something is a macro in this way, relative to the pain.  We're
probably talking about changes to 50% of the lines in many files in the
compiler.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [PATCH][RFC] Introduce type safety and debuggability into tree.h

[Diego Novillo]

On 8/9/07 4:55 PM, Mark Mitchell wrote:

> Yes, and, hence, we will always avoid the merge pain. :-)

Ah, sure, that's a good point :)

> I just don't see that there's that much advantage in advertising whether
> or not something is a macro in this way, relative to the pain.  We're
> probably talking about changes to 50% of the lines in many files in the
> compiler.

I agree.  I don't really have a strong preference for this.
Historically we've named these in capitals, the names are probably
pretty entrenched in people's minds.

Moreover, it probably simplifies Simon's patch quite a bit.  I'll go
with the popular vote here.  I don't really mind.

Re: [PATCH][RFC] Introduce type safety and debuggability into tree.h

[Kaveh R. GHAZI]

On Thu, 9 Aug 2007, Simon Baldwin wrote:

> This patch replaces some of the function-like macros in gcc/tree.h with
> static inlined function equivalents.

Thanks for using const_tree in the prototypes!

		--Kaveh
--
Kaveh R. Ghazi			ghazi@caip.rutgers.edu

Re: [PATCH][RFC] Introduce type safety and debuggability into tree.h

[Tom Tromey]

>>>>> "Simon" == Simon Baldwin <simonb@google.com> writes:

Simon> This patch replaces some of the function-like macros in gcc/tree.h
Simon> with static inlined function equivalents.

Simon> The aim is to improve the experience of debugging gcc with gdb, and to
Simon> introduce some type-safety into tree accessor functions.  

Nice.  For debugging it would be nice to get rid of all the statement
expressions (my #1 complaint when debugging gcc), but I suppose this
goes quite a long way toward that by making one less likely to hit
them with gdb.

Tom

RE: [PATCH][RFC] Introduce type safety and debuggability into tree.h

[Dave Korn]

On 09 August 2007 23:22, Simon Baldwin wrote:

> This patch replaces some of the function-like macros in gcc/tree.h with
> static inlined function equivalents.


  Won't this cause massive bloat in the case of bootstrapping (perhaps stage1
only?) which uses -fkeep-inline-functions (and when using non-GCC native
compilers, which don't necessarily eliminate static inlines)?  It might be
better to use something a bit more like ...

#if defined(__GNUC__) && !defined(__GNUC_STDC_INLINE__)
/* We're using GCC, but without the new C99-compatible behaviour.  */
#define _ELIDABLE_INLINE extern __inline__ _ATTRIBUTE ((__always_inline__))
#else
/* We're using GCC in C99 mode, or an unknown compiler which
  we just have to hope obeys the C99 semantics of inline.  */
#define _ELIDABLE_INLINE __inline__
#endif

(name not important, that's just what we used over in newlib).


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Re: [PATCH][RFC] Introduce type safety and debuggability into tree.h

[Michael Matz]

Hi,

On Thu, 9 Aug 2007, Mark Mitchell wrote:

> I do have one request about this patch: make it easier for yourself by 
> not changing the case of the macros.  That will avoid the mass changes 
> through all the other files, and will make life a lot easier for people 
> who backport or merge patches often.  (Yes, that's self-interested, but 
> I don't think it's just CodeSourcery that does that.)  Sure, the coding 
> standards may say that ALL_CAPS is for macros, but I just don't see the 
> value in this kind of mass change.

I completely agree with this.  Following guidelines has its value but that 
those predicates/accessors were written in upper case since the dawn of 
time also is sort of a defacto guideline.  We can't follow both, so we 
should follow the one which makes life easier and produces less disruption 
in the sources.  It's not like anyone would be confused to no end by 
seeing a uppercase function name for the things in tree.h.


Ciao,
Michael.

Re: [PATCH][RFC] Introduce type safety and debuggability into tree.h

[Richard Kenner]

> I completely agree with this.  Following guidelines has its value but that 
> those predicates/accessors were written in upper case since the dawn of 
> time also is sort of a defacto guideline.  We can't follow both, so we 
> should follow the one which makes life easier and produces less disruption 
> in the sources.  It's not like anyone would be confused to no end by 
> seeing a uppercase function name for the things in tree.h.

I agree as well, for the same reason.