This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

gengtype improvements for plugins, completed! patch 0/N


Hello All,

Join work by Basile Starynkevitch & Jeremie Salvucci, taking into
account the feedback we got previously.

See http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02047.html and many
other mails, notably
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02069.html [whichs list
more of them]

I've finally got the bug in our gengtype-state.c file. For Jeremie (who
is currently in holidays for only a week) and other people curious
about the bug we spent a month to track, the next field of type_p
structures inside gengtype has two uses. First and mostly, it makes a
big list (going into the structures variable) of all the GTY-ed struct
& union-s. There is also a param_structs variable which is a smaller
linked list of parametrized GTY-ed types. But the next field is also
used within lang_struct-s to make a linked list of homonymous types
which are specific to a given GCC front-end language. 

Honestly, gengtype is a very dirty code.  Perhaps it might be the first
piece of GCC to be recoded in C++ (I don't like much C++, but I do
admit that its container templates would help a lot to make gengtype
cleaner).  Of course, this patch serie is not a rewriting of gengype in
C++!


We (Jeremie & Basile) now have a patch which is able to persist
gengtype state in a textual file which can be used in plugin mode. So
if our serie of patches is accepted in 4.6, plugins could use GTY
without requiring (as the 4.5 kludge) availability of GCC source and
build trees.

I just uploaded the entire work as one big diff on
http://starynkevitch.net/Basile/gengtype-r163961-07sept2010.diff - if
people want to test the code, they could apply that as a big patch
against trunk 163961. At least, that big patch demonstrates that a
persistent gengtype is doable. In addition, if I am hit by a bus
tommorrow, perhaps some bright people could do what I am starting now
again: splitting it into manageable pieces. There are some dirt
remaining there (in particular some #warning which I am removing right
now).

However, I am very scared and even a bit discouraged.  The previous
patches got no comments by any person able to OK them.  It seems that
no reviewer is interested in reviewing this work.  If nobody reviews
it, how is it possible to push it into 4.6?  I have the very
uncomfortable feeling that people find the goals of this patch
interesting, but nobody understand well enough gengtype and have enough
time to review it and OK it.  So far, all the few nice comments I've
got have been from people not authorized or not willing to review that
patch and give enough feedback or an OK to commit it. The person most
interested in this serie of patches is probably Laurynas Biveinis, and
I thank him warmly for his feeedback & support.  Unfortunately, he has
no authority to OK any commit!  I don't understand how can that work be
pushed into 4.6 if no reviewer cares! And 4.6 stage 1 is ending
soon!!!   I do understand however, why no reviewer is interested:
first, this patch is only really useful for plugins using GTY (and I
know of no such plugin except my MELT; and perhaps many old GCC people
are still against plugins.). Second, gengtype is really crappy code,
and few people understand it well (otherwise, some other people than
Laurynas would have answered my technical questions). And third, this
patch does not improve the code produced by GCC, since it does not
change it at all, since all the gengtype generated C code is not
changed by this work.

I will now splice again that patch in several chunks, perhaps improving
slightly our code during this splitting. I will try to take into
account every comment we got so far. If I did forgot a comment please
tell.



The intended splits are as before (except that we now add an entirely
working gengtype-state.c file). I gave a mnemotechnic name to each
patch in brackets:

patch 1 [declprog], like
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02047.html moves many
private definitions, types, functions from gengtype.c to gengtype.h &
provide a GNU friendly gengtype program invocation. We have to declare
publicly most internal types of gengtype since we really want the
persistent machinery to be in a separate file gengtype-state.c and
because we feel that gengtype.c is messy and big enough.

patch 2 [verbosity], like
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02060.html, add a verbose
flag -v to the gengtype program. When given (it can be given more than
once to increase verbosity), it explains what gengtype is doing. This
flag should be useful to any gengtype user.

patch 3 [inputfile], like
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02063.html provides an
input_file structure remove the horrible disgusting hack of encoding
the set of languages in a bitmap in four bytes before the file path.
Honestly, I cannot understand why the old gengtype had such an horrible
coding, and how could it have been accepted in the first place, nearly
nine or ten years ago.

patch 4 [filesrules] like
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02065.html provides a
better machinery to associate an output file to each input_file. So it
is improving the get_output_file_with_visibility function with a rule &
regular-expression based machinery.  I did thought a few seconds about
adding a convention in comments -for instance requiring that gimple.h
has a comment /*@@ GENGTYPEOUTPUT gt-gimple.h */, and tree.h have /*@@
GENGTYPEOUTPUT gt-tree.h */ etc, and have gengtype parse such comments.
However, such a patch would require me to patch, by adding just a
comment, almost every file of GCC, and I am sadly pretty sure nobody
will review such a work; the social rules of GCC make such an idea
impractical.

patch 5 [typedopt] like
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02068.html is making each
option a disciminated union. This is needed to be able to persist
gengtype state. 

patch 6 [wstate] see
http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02069.html gives the
gengtype-state.c which has all the persistency machinery and enable its
use.

patch 7 [doc] improves the documentation gty.texi

Maybe another patch will send some more improvements. In particular, I
want to check that mark_hook GTY is still working, and I would like to
send later a small patch which adds conditional #if in the generated
marking code.

Maybe a convention should be firmly defined of how the gengtype program
is named (perhaps gcc-gengtype is better) and where is the default
state file (perhaps in the  plugins directory).


As a general rule, Jeremie & me Basile find gengtype to be of poor
quality.  We prefer much to increase its readability & maintainability,
even if it runs a bit slower. We feel that making gengtype easier to
debug & to enhance is much more important than its speed.  Remember
that gengtype is run very rarely in practice!

By the way, I have the feeling that unchanged trunk rev.163961 crashes:

I am building it on a AMD64 Debian testing
in /usr/src/Lang/_GccTrunkObj build tree
and  /usr/src/Lang/gcc-trunk-bstarynk source tree. Configure options are

 '/usr/src/Lang/gcc-trunk-bstarynk/configure'   '--enable-plugins'
'--program-suffix=-plaintrunk' '--enable-maintainer-mode'
'--enable-checks=tree,gc' '--disable-bootstrap' '--disable-multilib'
'--enable-version-specific-runtime-libs' 'CFLAGS=-g'
'--enable-languages=c,c++,lto,objc' 

The build fails with:
 /usr/src/Lang/gcc-trunk-bstarynk/libgcc/../gcc/libgcc2.c: In function
'__multi3': /usr/src/Lang/gcc-trunk-bstarynk/libgcc/../gcc/libgcc2.c:558:1:
internal compiler error: Segmentation fault


My patch crashes at the same moment since it generates the same gt*.
[ch] files!


The script below is used to check that the gengtype generated files are
same in trunk and with the patch.

########################### comparegengtypefiles script ###############################
#! /bin/bash
## The trunk build directory
gcctrunkobjdir=$1

## The patched GCC build directory
gccgengtypeobjdir=$2

if [ ! -d "$gcctrunkobjdir" ]; then
   echo $0: no Trunk obj dir 1st arg > /dev/stderr
   exit 1
fi

if [ ! -d "$gccgengtypeobjdir" ]; then
   echo $0: no Gengtype obj dir 2nd arg > /dev/stderr
   exit 1
fi

if [ ! -x $gcctrunkobjdir/build/gengtype ]; then
   echo $0: no $gcctrunkobjdir/build/gengtype > /dev/stderr
   exit 1
fi

if [ ! -x $gccgengtypeobjdir/build/gengtype ]; then
   echo $0: no $gccgengtypeobjdir/build/gengtype > /dev/stderr
   exit 1
fi

if [ ! -f $gcctrunkobjdir/gtype-desc.c ]; then
   echo $0: no $gcctrunkobjdir/gtype-desc.c > /dev/stderr
   exit 1
fi
if [ ! -f $gccgengtypeobjdir/gtype-desc.c ]; then
   echo $0: no $gccgengtypeobjdir/gtype-desc.c > /dev/stderr
   exit 1
fi

gtfiles=$(cd $gcctrunkobjdir; echo gt*.[ch]) 
noteqfiles=""
samefiles=''
for f in $gtfiles; do
    (diff -u -b  $gcctrunkobjdir/$f $gccgengtypeobjdir/$f | head -21)
    if cmp $gcctrunkobjdir/$f $gccgengtypeobjdir/$f ; then
      samefiles="$samefiles $f"
    else
       noteqfiles="$noteqfiles $f"
       echo 
    fi
done

echo $0 same files= $samefiles

#################################################################################

I still hope our patches will be reviewed by someone authorized to send
an OK when they will be ready enough.  I also hope that 4.6 will be
released with our patches (suitably improved) included! But I am very
scared that no reviewer really care about that. What is the correct
frequency to ping  such a serie of patches? Who, amongst the reviewers,
is particularily interested or motivated to review that work? If you
are, please tell!

Cheers.

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]