This is the mail archive of the gcc@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]

Improving gengtype (for plugin support notably) - how to get a rather big patch accepted?


Hello All

Jeremie Salvucci and me Basile are working on improving gengtype. Our
patch is still buggy [curious people might retrieve it from
http://starynkevitch.net/Basile/gengtype-r163335-24august-2010.diff but
I will remove that 267Kytes file of 9278 lines in a few days, and that
file is a *buggy* patch against trunk r163335 and it should be cleaned &
corrected before submission] and not ready yet for proposal to
gcc-patches@ Today, our patch is still buggy (Jeremie worked more than a
month on it, and I did work a few weeks on it with him) and not entirely
complete.

However, both Jeremie and me are very scared that, even when it will be
ready and sent to gcc-patches@, our patch will not be reviewed by people
legitimately able to OK it and will be completely ignored (actually,
this happened to us before e.g.
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02178.html etc).

It is extremely important for us (and particularly for me Basile & for
GCC MELT) to have this patch accepted in 4.6, since it enables plugin to
use GTY & the Ggc garbage collector. So far, the only way a plugin [like
MELT] could add its own GTY-ed data is by running gengtype with both the
GCC build and source entire trees, a requirement against every packaging
principles and very uncommon (As Diego told once, it is a "dirty hack").

We were kindly told in private emails (from Diego & from Laurynas) that
to increase the chances to get a patch accepted, it should be sliced in
small pieces.

However, I don't understand well how practically we could slice our
patches (especially since the small
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02178.html  has been
essentially ignored, which gives me the impression that few people care
about gengtype; so no matter if a gengtype patch is large or small, I am
afraid it won't be reviewed!).

First, a few technical questions:

*  what is a sequence of related small patches? Is it an ordered set of
patches to apply in succession? By what exact set of commands (assuming
a GNU/Linux system). Does a latter patch in that sequence apply to the
trunk, or to the trunk updated by previous patches?

* what is the preferred way of obtaining a sequence of small patches?
svn diff -x -p gives one big *.diff file! Should we split it by hand?
Are there other tools producing a sequence of small patches?



Then, more specifically about our gengtype work.

The bulk of our patch is a quite big set of routines to write the state
of gengtype into a textual file, and to read it again. We ensure that
the state is well persisted by having the build process first write the
state file using
   build/gengtype -S $(srcdir) -I gtyp-input.list -w gtype.state
this command just write gtype.state file of a bit less than 800Kbytes.
It is a textual file in Lispy syntax (its lexer is taken from MELT).

Then the build process reads that gtype.state file and generates all the
usual gengtype-generated files without reading any other [e.g. GCC
source] file [today we still have bugs!] by running
  build/gengtype -r gtype.state
This should produce the same behavior that current trunk's gengtype. In
other words, our gengtype generates exactly the same code as today!

An hypothetical plugin would probably run
   gengtype -r gtype.state -P gt-plugin.h plugin*.c
[perhaps some more options would be needed]
and the gtype.state file will actually be installed in the plugins
directory or in some other GCC installation subdirectory.

I hope people do understand that our main motivation is to get plugin
support in gengtype into gcc-4.6.

The main way I see to split our patch in two pieces is to have a new
file gengtype-state.c which would contain all the state reading &
writing routines.

However, our patch also added some improvements to gengtype itself
(honestly, both Jeremie & me believe that gengtype.c is the worse source
code we ever studied inside GCC; its quality seems much lower that that
of the main GCC part, i.e. the middle-end files). We had to add minor
improvements such as

  * the ability to pass program options to GCC. That is exactly the
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02178.html and following
patch, and it has been ignored.

  * we removed the ugly hack that the set of languages is hardcoded in
file path strings, just before the string itself. Now, input_file is a
structure containing both the bitmask and the file name.

  * we replaced the clumsy sequence of string compares in
get_output_file_with_visibility to compute the output file for a given
input file by a regular-expression rules machinery. We now have a small
sequence of rules, each containing a regular expression to match the
input file name, and replacement strings for output_name & for_name
files, and an additional action routine to improve or replace them.

  * we cleaned up a little bit some code pieces that we did not
understand (but the generated C code is untouched).

However, most of our changes are related and I am not sure we could post
meaningful, independent patches. What kind of patch slicing do you
concretely advise?

Could some GCC reviewers authorized & willing to review and latter to OK
a patch on gengtype give us some concrete advices on how to get such a
patch accepted? Who is a reviewer able to review our patch when it will
be ready [the current state of our patch is buggy & not satisfactory],
and having enough time for that? Who should we contact first? Laurynas
knows very well gengtype but is not a reviewer. Diego probably is
overworked.

Any additional advice is welcome.

Please notice that even if this is a big patch, it does not [in
principle] change the behavior of gengtype as observed from GCC code,
since no generated file change! So our patch does not change anything in
the resulting GCC compiler executables (e.g. cc1 etc...). I feel that
this should ease the acceptance of our patch when it would be ready.

Please remember that English is not our native language, and that we are
culturally & geographically very far away from most of [North-American]
GCC developers (and have different timezone, ours is MEST = GMT+2hours
today).

We feel that our patch should get into 4.6 otherwise it would still be
impractical to build plugins needing GTY.

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]