PATCH GCC5.0: conditionally skip gcc_version in gcc-plugin.h

PATCH GCC5.0: conditionally skip gcc_version in gcc-plugin.h

[Basile Starynkevitch]

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

Hello All,

Some plugins (including MELT, see http://gcc-melt.org/ for more)
are made of several C++ source files which all include "plugin-version.h"
because they have some C++ code which depends upon the particular version 
of GCC.

So they typically code

  #if GCCPLUGIN_VERSION >= 4009
     /* code for GCC 4.9 or newer. */
  #else 
     /* code for GCC 4.8 */
  #endif /*GCCPLUGIN_VERSION*/

after including "plugin-version.h"; however that file also defines static
data, notably `gcc_version`. That data symbol may be useless in most of 
the plugin files -except the one initializing the plugin. Having several
useless data symbols may disturb the debugger (since the static symbol
`gcc_version` is no longer unique) and may consume some tiny useless data
(at least when the plugin is compiled with -O0).

The attached small patch (for trunk svn rev. 217404) disables the definition 
of `gcc_version` when the preprocessor symbol GCCPLUGIN_SKIP_VERSION_DATA 
is defined as 1 before #include "plugin-version.h"

### gcc/ChangeLog entry:

2014-11-12  Basile Starynkevitch  <basile@starynkevitch.net>

	* configure.ac (plugin-version.h): Don't define version data 
	when GCCPLUGIN_SKIP_VERSION_DATA was #define-d as 1.

	* doc/plugins.texi: (Plugins building): Document 
	GCCPLUGIN_SKIP_VERSION_DATA. Put it with GCCPLUGIN_VERSION* names
	in the function index.

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

Ok for trunk? Comments are welcome.

Regards

-- 
Basile Starynkevitch     http://starynkevitch.net/Basile

[-- Attachment #2: gcc5-r217404-gccpluginversion-skip.diff --]
[-- Type: text/x-diff, Size: 1984 bytes --]

Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 217404)
+++ gcc/configure.ac	(working copy)
@@ -1664,6 +1664,10 @@
 #define GCCPLUGIN_VERSION_PATCHLEVEL   `echo $gcc_BASEVER | sed -e 's/^[0-9]*\.[0-9]*\.\([0-9]*\)$/\1/'`
 #define GCCPLUGIN_VERSION  (GCCPLUGIN_VERSION_MAJOR*1000 + GCCPLUGIN_VERSION_MINOR)
 
+/* Some plugins might not want the data below, they would define
+   GCCPLUGIN_SKIP_VERSION_DATA as 1 before including this.  */
+
+#if !GCCPLUGIN_SKIP_VERSION_DATA
 static char basever[] = "$gcc_BASEVER";
 static char datestamp[] = "$gcc_DATESTAMP";
 static char devphase[] = "$gcc_DEVPHASE";
@@ -1675,6 +1679,7 @@
 static struct plugin_gcc_version gcc_version = {basever, datestamp,
 						devphase, revision,
 						configuration_arguments};
+#endif /* GCCPLUGIN_SKIP_VERSION_DATA */
 EOF
 changequote([,])dnl
 
Index: gcc/doc/plugins.texi
===================================================================
--- gcc/doc/plugins.texi	(revision 217404)
+++ gcc/doc/plugins.texi	(working copy)
@@ -157,6 +157,16 @@
 
 but you can also check the individual fields if you want a less strict check.
 
+A plugin might want to include in some of its source files the
+@file{plugin-version.h} header for preprocessor constants
+@code{GCCPLUGIN_VERSION} without defining the static symbol
+@code{gcc_version}. In that case it should define the preprocessor
+symbol @code{GCCPLUGIN_SKIP_VERSION_DATA} to @code{1} before including
+that header.
+@findex GCCPLUGIN_VERSION
+@findex GCCPLUGIN_SKIP_VERSION_DATA
+@findex gcc_version
+
 @subsection Plugin callbacks
 
 Callback functions have the following prototype:
@@ -488,6 +498,10 @@
 #error this GCC plugin is for GCC 4.7
 #endif
 @end smallexample
+@findex GCCPLUGIN_VERSION_MAJOR 
+@findex GCCPLUGIN_VERSION_MINOR
+@findex GCCPLUGIN_VERSION_PATCHLEVEL
+@findex GCCPLUGIN_VERSION
 
 The following GNU Makefile excerpt shows how to build a simple plugin:
 

Re: PATCH GCC5.0: conditionally skip gcc_version in gcc-plugin.h

[Jakub Jelinek]

On Wed, Nov 12, 2014 at 01:55:12PM +0100, Basile Starynkevitch wrote:
> Hello All,
> 
> Some plugins (including MELT, see http://gcc-melt.org/ for more)
> are made of several C++ source files which all include "plugin-version.h"
> because they have some C++ code which depends upon the particular version 
> of GCC.
> 
> So they typically code
> 
>   #if GCCPLUGIN_VERSION >= 4009
>      /* code for GCC 4.9 or newer. */
>   #else 
>      /* code for GCC 4.8 */
>   #endif /*GCCPLUGIN_VERSION*/

Can't you just remember that version in configure of your plugin?

	Jakub

Re: PATCH GCC5.0: conditionally skip gcc_version in plugin-version.h

[Basile Starynkevitch]

On Wed, Nov 12, 2014 at 02:12:07PM +0100, Jakub Jelinek wrote:
> On Wed, Nov 12, 2014 at 01:55:12PM +0100, Basile Starynkevitch wrote:
> > Hello All,
> > 
> > Some plugins (including MELT, see http://gcc-melt.org/ for more)
> > are made of several C++ source files which all include "plugin-version.h"
> > because they have some C++ code which depends upon the particular version 
> > of GCC.
> > 
> > So they typically code
> > 
> >   #if GCCPLUGIN_VERSION >= 4009
> >      /* code for GCC 4.9 or newer. */
> >   #else 
> >      /* code for GCC 4.8 */
> >   #endif /*GCCPLUGIN_VERSION*/
> 
> Can't you just remember that version in configure of your plugin?


Most plugin don't need any configure, because they are installed in 
a version specific directory (like /usr/lib/gcc/x86_64-linux-gnu/4.9/plugin 
for example). I don't think it is wise to require plugin to be 
autoconf-configurable. Their Makefile simply uses 
$(shell gcc -print-file-name=plugin), there is no need to complex
autoconf machinery.

And even a plugin for a particular version of GCC is usually made
of several files, all of them with #include "plugin-version.h";
there is no need to define several times gcc_version.

(another possibility might be to make gcc_version an external symbol
with public visibility inside the cc1 or cc1plus executable)

Thanks for your comment.

Cheers. 
--
Basile Starynkevitch        http://starynkevitch.net/Basile/

Re: PATCH GCC5.0: conditionally skip gcc_version in plugin-version.h

[Jakub Jelinek]

On Wed, Nov 12, 2014 at 02:20:22PM +0100, Basile Starynkevitch wrote:
> Most plugin don't need any configure, because they are installed in 
> a version specific directory (like /usr/lib/gcc/x86_64-linux-gnu/4.9/plugin 
> for example). I don't think it is wise to require plugin to be 
> autoconf-configurable. Their Makefile simply uses 
> $(shell gcc -print-file-name=plugin), there is no need to complex
> autoconf machinery.

If you use $(shell gcc -print-file-name=plugin), there is no point
to include plugin-version.h, just use __GNUC__/__GNUC_MINOR__ ?

	Jakub

Re: PATCH GCC5.0: conditionally skip gcc_version in plugin-version.h

[Basile Starynkevitch]

On Wed, Nov 12, 2014 at 02:29:13PM +0100, Jakub Jelinek wrote:
> On Wed, Nov 12, 2014 at 02:20:22PM +0100, Basile Starynkevitch wrote:
> > Most plugin don't need any configure, because they are installed in 
> > a version specific directory (like /usr/lib/gcc/x86_64-linux-gnu/4.9/plugin 
> > for example). I don't think it is wise to require plugin to be 
> > autoconf-configurable. Their Makefile simply uses 
> > $(shell gcc -print-file-name=plugin), there is no need to complex
> > autoconf machinery.
> 
> If you use $(shell gcc -print-file-name=plugin), there is no point
> to include plugin-version.h, just use __GNUC__/__GNUC_MINOR__ ?


I could compile a plugin (notably for a GCC cross-compiler) with a GCC version
different of the GCC targetting the plugin. I could also compile a 
plugin with Clang or some other non-GCC compiler. In both cases
plugin-version.h is needed with its GCCPLUGIN_VERSION.

Cheers.

--
Basile Starynkevitch     http://starynkevitch.net/Basile/

Re: PATCH GCC5.0: conditionally skip gcc_version in gcc-plugin.h

[Richard Biener]

On Wed, Nov 12, 2014 at 2:12 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Nov 12, 2014 at 01:55:12PM +0100, Basile Starynkevitch wrote:
>> Hello All,
>>
>> Some plugins (including MELT, see http://gcc-melt.org/ for more)
>> are made of several C++ source files which all include "plugin-version.h"
>> because they have some C++ code which depends upon the particular version
>> of GCC.
>>
>> So they typically code
>>
>>   #if GCCPLUGIN_VERSION >= 4009
>>      /* code for GCC 4.9 or newer. */
>>   #else
>>      /* code for GCC 4.8 */
>>   #endif /*GCCPLUGIN_VERSION*/
>
> Can't you just remember that version in configure of your plugin?

Or fix it properly by also generating a plugin-version.c file with
the definitions and just provide declarations in plugin-version.h.

Richard.

>         Jakub

Re: PATCH GCC5.0: conditionally skip gcc_version in gcc-plugin.h

[Richard Biener]

On Wed, Nov 12, 2014 at 3:35 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Nov 12, 2014 at 2:12 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Wed, Nov 12, 2014 at 01:55:12PM +0100, Basile Starynkevitch wrote:
>>> Hello All,
>>>
>>> Some plugins (including MELT, see http://gcc-melt.org/ for more)
>>> are made of several C++ source files which all include "plugin-version.h"
>>> because they have some C++ code which depends upon the particular version
>>> of GCC.
>>>
>>> So they typically code
>>>
>>>   #if GCCPLUGIN_VERSION >= 4009
>>>      /* code for GCC 4.9 or newer. */
>>>   #else
>>>      /* code for GCC 4.8 */
>>>   #endif /*GCCPLUGIN_VERSION*/
>>
>> Can't you just remember that version in configure of your plugin?
>
> Or fix it properly by also generating a plugin-version.c file with
> the definitions and just provide declarations in plugin-version.h.

And of course not export the too unspecific symbols 'basever'
'datestamp' 'devphase' and 'revision'.  Only export gcc_version.
Which is already available from 'version_string' in version.[ch](!?).  So what's
the point of the extra stuff in plugin-version.h??

Richard.

> Richard.
>
>>         Jakub

Re: PATCH GCC5.0: conditionally skip gcc_version in plugin-version.h

[Basile Starynkevitch]

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

On Wed, 2014-11-12 at 14:36 +0100, Basile Starynkevitch wrote:
> On Wed, Nov 12, 2014 at 02:29:13PM +0100, Jakub Jelinek wrote:
> > On Wed, Nov 12, 2014 at 02:20:22PM +0100, Basile Starynkevitch wrote:
> > > Most plugin don't need any configure, because they are installed in 
> > > a version specific directory (like /usr/lib/gcc/x86_64-linux-gnu/4.9/plugin 
> > > for example). I don't think it is wise to require plugin to be 
> > > autoconf-configurable. Their Makefile simply uses 
> > > $(shell gcc -print-file-name=plugin), there is no need to complex
> > > autoconf machinery.
> > 
> > If you use $(shell gcc -print-file-name=plugin), there is no point
> > to include plugin-version.h, just use __GNUC__/__GNUC_MINOR__ ?
> 
> 
> I could compile a plugin (notably for a GCC cross-compiler) with a GCC version
> different of the GCC targetting the plugin. I could also compile a 
> plugin with Clang or some other non-GCC compiler. In both cases
> plugin-version.h is needed with its GCCPLUGIN_VERSION.

I'm trying to patch GCC to get a plugin-version.c file generated, but I
can't get that work. Here is attached a buggy patch against trunk svn
r217460 which does not work. Could any one help me to catch my mistake
please?


Regards.

-- 
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} ***


[-- Attachment #2: badpatch-gcc-version.diff --]
[-- Type: text/x-patch, Size: 2799 bytes --]

Index: fixincludes/fixincl.x
===================================================================
--- fixincludes/fixincl.x	(revision 217460)
+++ fixincludes/fixincl.x	(working copy)
@@ -1,12 +1,12 @@
 /*  -*- buffer-read-only: t -*- vi: set ro:
- * 
+ *
  * DO NOT EDIT THIS FILE   (fixincl.x)
- * 
- * It has been AutoGen-ed  October 21, 2014 at 10:18:16 AM by AutoGen 5.16.2
+ *
+ * It has been AutoGen-ed
  * From the definitions    inclhack.def
  * and the template file   fixincl
  */
-/* DO NOT SVN-MERGE THIS FILE, EITHER Tue Oct 21 10:18:17 CEST 2014
+/* DO NOT SVN-MERGE THIS FILE, EITHER Thu Nov 13 07:50:38 MET 2014
  *
  * You must regenerate it.  Use the ./genfixes script.
  *
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	(revision 217460)
+++ gcc/Makefile.in	(working copy)
@@ -1324,6 +1324,7 @@
 	opts-global.o \
 	passes.o \
 	plugin.o \
+	plugin-version.o \
 	postreload-gcse.o \
 	postreload.o \
 	predict.o \
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 217460)
+++ gcc/configure.ac	(working copy)
@@ -1656,7 +1656,25 @@
 else
         gcc_REVISION=""
 fi
+
+cat > plugin-version.c <<EOF
+/* generated file plugin-version.c for GCC */
+#include "plugin-version.h"
+static const char basever[] = "$gcc_BASEVER";
+static const char datestamp[] = "$gcc_DATESTAMP";
+static const char devphase[] = "$gcc_DEVPHASE";
+static const char revision[] = "$gcc_REVISION";
+
+/* FIXME plugins: We should make the version information more precise.
+   One way to do is to add a checksum. */
+
+const struct plugin_gcc_version gcc_version = {basever, datestamp,
+                                               devphase, revision,
+                                               configuration_arguments};
+EOF
+
 cat > plugin-version.h <<EOF
+/* generated header plugin-version.h for GCC */
 #include "configargs.h"
 
 #define GCCPLUGIN_VERSION_MAJOR   `echo $gcc_BASEVER | sed -e 's/^\([0-9]*\).*$/\1/'`
@@ -1664,18 +1682,9 @@
 #define GCCPLUGIN_VERSION_PATCHLEVEL   `echo $gcc_BASEVER | sed -e 's/^[0-9]*\.[0-9]*\.\([0-9]*\)$/\1/'`
 #define GCCPLUGIN_VERSION  (GCCPLUGIN_VERSION_MAJOR*1000 + GCCPLUGIN_VERSION_MINOR)
 
-static char basever[] = "$gcc_BASEVER";
-static char datestamp[] = "$gcc_DATESTAMP";
-static char devphase[] = "$gcc_DEVPHASE";
-static char revision[] = "$gcc_REVISION";
+extern const struct plugin_gcc_version gcc_version;
+EOF
 
-/* FIXME plugins: We should make the version information more precise.
-   One way to do is to add a checksum. */
-
-static struct plugin_gcc_version gcc_version = {basever, datestamp,
-						devphase, revision,
-						configuration_arguments};
-EOF
 changequote([,])dnl
 
 # Internationalization