Re: [RFC 2/5] gcc: xtensa: make configuration dynamic

2017-05-26 Thread Max Filippov
On Fri, May 26, 2017 at 7:44 AM, Ian Lance Taylor  wrote:
> On Thu, May 25, 2017 at 1:31 PM, Max Filippov  wrote:
>> On Thu, May 25, 2017 at 11:24 AM, augustine.sterl...@gmail.com
>>  wrote:
>>
>>> Please note that by using a plugin mechanism, there are licensing
>>> issues that come into play, that are different from the usual
>>> licensing issues. I would be absolutely sure that you all are OK with
>>> how the runtime exception applies to this new situation.
>>
>> All code used for building the configuration shared object is either GPL
>> (part of binutils) or MIT (xtensa configuration overlay), so it should be ok?
>
> You are in effect introducing a new kind of plugin mechanism.  I won't
> comment on whether it should use the existing plugin mechanism or not,
> but it's important to stress that the GCC Runtime Library Exception
> (https://www.gnu.org/licenses/gcc-exception-3.1.en.html) has rules
> that apply here.  In effect, if you want to distribute the binary
> produced by GCC, all the plugins that you use must be available under
> a GPL-compatible license.  The people to whom you distribute the
> binary produced by GCC must be able to themselves build the plugin
> used to create the binary.  The plugin may not have any proprietary
> source code.

Thanks, that's how I understood that. I've added a note to the plugin
README: https://github.com/jcmvbkbc/xtensa-dynconfig/blob/master/README

> One way that the GCC plugin mechanism makes that clear is by requiring
> the plugin to define a symbol named, literally,
> "plugin_is_GPL_compatible".  While there is no enforcement mechanism
> as such, this ensures that the person creating the plugin acknowledges
> that at the very least the plugin is supposed to be under a GPL
> compatible license.  I think that if you are going to introduce a new
> plugin mechanism, you should adopt the same approach.

Sure I can add such requirement to this mechanism and implement that
check.

-- 
Thanks.
-- Max


Re: [RFC 2/5] gcc: xtensa: make configuration dynamic

2017-05-26 Thread Ian Lance Taylor via gcc-patches
On Thu, May 25, 2017 at 1:31 PM, Max Filippov  wrote:
> On Thu, May 25, 2017 at 11:24 AM, augustine.sterl...@gmail.com
>  wrote:
>
>> Please note that by using a plugin mechanism, there are licensing
>> issues that come into play, that are different from the usual
>> licensing issues. I would be absolutely sure that you all are OK with
>> how the runtime exception applies to this new situation.
>
> All code used for building the configuration shared object is either GPL
> (part of binutils) or MIT (xtensa configuration overlay), so it should be ok?

You are in effect introducing a new kind of plugin mechanism.  I won't
comment on whether it should use the existing plugin mechanism or not,
but it's important to stress that the GCC Runtime Library Exception
(https://www.gnu.org/licenses/gcc-exception-3.1.en.html) has rules
that apply here.  In effect, if you want to distribute the binary
produced by GCC, all the plugins that you use must be available under
a GPL-compatible license.  The people to whom you distribute the
binary produced by GCC must be able to themselves build the plugin
used to create the binary.  The plugin may not have any proprietary
source code.

One way that the GCC plugin mechanism makes that clear is by requiring
the plugin to define a symbol named, literally,
"plugin_is_GPL_compatible".  While there is no enforcement mechanism
as such, this ensures that the person creating the plugin acknowledges
that at the very least the plugin is supposed to be under a GPL
compatible license.  I think that if you are going to introduce a new
plugin mechanism, you should adopt the same approach.

Ian


Re: [RFC 2/5] gcc: xtensa: make configuration dynamic

2017-05-25 Thread Max Filippov
On Thu, May 25, 2017 at 11:24 AM, augustine.sterl...@gmail.com
 wrote:
> On Mon, May 22, 2017 at 2:09 PM, Max Filippov  wrote:
>> Now that XCHAL_* macros don't have to be preprocessor constants add
>> include/xtensa-dynconfig.h that defines them as fields of a structure
>> returned from the xtensa_get_config function.
>> Define that structure and fill it with default parameter values
>> specified in the include/xtensa-config.h.
>> Define reusable function xtensa_load_config that tries to load
>> configuration and return an address of an exported object from it.
>> Define the function xtensa_get_config that uses xtensa_load_config to
>> get structure xtensa_config, either dynamically configured or the
>> default.
>>
>> 2017-05-22  Max Filippov  
>> gcc/
>> * Makefile.in (PLUGIN_HEADERS): Add include/xtensa-dynconfig.h.
>> * config.gcc (xtensa*-*-*): Add xtensa-config.o to extra_objs.
>> * gcc/config/xtensa/t-xtensa (xtensa-config.o): New rule.
>> * gcc/config/xtensa/xtensa-config.c: New file.
>> * gcc/config/xtensa/xtensa.h (xtensa-config.h): Replace #include
>> with xtensa-dynconfig.h
>> (XCHAL_HAVE_MUL32_HIGH, XCHAL_HAVE_RELEASE_SYNC,
>>  XCHAL_HAVE_S32C1I, XCHAL_HAVE_THREADPTR,
>>  XCHAL_HAVE_FP_POSTINC): Drop definitions.
>
> This almost certainly should go through the normal gcc plugin
> mechanism instead of the hand-rolled one you have here.
>
> https://gcc.gnu.org/onlinedocs/gccint/Plugins.html#Plugins
>
> If there is a reason it can't (and I'm not sufficiently familiar with
> the issues here), then you need to ensure that the various protections
> enforced by the normal plugin mechanism is used--and someone more
> knowledgeable than me needs to review it.

(adding Le-Chun Wu, current reviewer of the plugin code)

I tried to use plugins initially, but the following considerations made me
lean towards this implementation:
- we don't actually need any of the functionality available to gcc plugins,
  we only need to provide a data structure that the compiler knows how
  to use;
- using environment variable makes configuration transparent for gcc, all
  binutils and gdb, whereas using gcc plugin would require changing gcc
  invocation command line, which is not always possible;
- this implementation is shared between gcc and binutils. OTOH using
  plugin with gcc suggests using plugin with binutils, where it's substantially
  different and not universally supported across different binutils components
  (particularly neither assembler nor objdump have any support for it);
- gcc does not support plugins when built for windows host using MinGW;

Does the above justify this implementation? If no, are there any suggestions
how the above points may be addressed?

> Please note that by using a plugin mechanism, there are licensing
> issues that come into play, that are different from the usual
> licensing issues. I would be absolutely sure that you all are OK with
> how the runtime exception applies to this new situation.

All code used for building the configuration shared object is either GPL
(part of binutils) or MIT (xtensa configuration overlay), so it should be ok?

-- 
Thanks.
-- Max


Re: [RFC 2/5] gcc: xtensa: make configuration dynamic

2017-05-25 Thread augustine.sterl...@gmail.com
On Mon, May 22, 2017 at 2:09 PM, Max Filippov  wrote:
> Now that XCHAL_* macros don't have to be preprocessor constants add
> include/xtensa-dynconfig.h that defines them as fields of a structure
> returned from the xtensa_get_config function.
> Define that structure and fill it with default parameter values
> specified in the include/xtensa-config.h.
> Define reusable function xtensa_load_config that tries to load
> configuration and return an address of an exported object from it.
> Define the function xtensa_get_config that uses xtensa_load_config to
> get structure xtensa_config, either dynamically configured or the
> default.
>
> 2017-05-22  Max Filippov  
> gcc/
> * Makefile.in (PLUGIN_HEADERS): Add include/xtensa-dynconfig.h.
> * config.gcc (xtensa*-*-*): Add xtensa-config.o to extra_objs.
> * gcc/config/xtensa/t-xtensa (xtensa-config.o): New rule.
> * gcc/config/xtensa/xtensa-config.c: New file.
> * gcc/config/xtensa/xtensa.h (xtensa-config.h): Replace #include
> with xtensa-dynconfig.h
> (XCHAL_HAVE_MUL32_HIGH, XCHAL_HAVE_RELEASE_SYNC,
>  XCHAL_HAVE_S32C1I, XCHAL_HAVE_THREADPTR,
>  XCHAL_HAVE_FP_POSTINC): Drop definitions.

This almost certainly should go through the normal gcc plugin
mechanism instead of the hand-rolled one you have here.

https://gcc.gnu.org/onlinedocs/gccint/Plugins.html#Plugins

If there is a reason it can't (and I'm not sufficiently familiar with
the issues here), then you need to ensure that the various protections
enforced by the normal plugin mechanism is used--and someone more
knowledgeable than me needs to review it.

Please note that by using a plugin mechanism, there are licensing
issues that come into play, that are different from the usual
licensing issues. I would be absolutely sure that you all are OK with
how the runtime exception applies to this new situation.


[RFC 2/5] gcc: xtensa: make configuration dynamic

2017-05-22 Thread Max Filippov
Now that XCHAL_* macros don't have to be preprocessor constants add
include/xtensa-dynconfig.h that defines them as fields of a structure
returned from the xtensa_get_config function.
Define that structure and fill it with default parameter values
specified in the include/xtensa-config.h.
Define reusable function xtensa_load_config that tries to load
configuration and return an address of an exported object from it.
Define the function xtensa_get_config that uses xtensa_load_config to
get structure xtensa_config, either dynamically configured or the
default.

2017-05-22  Max Filippov  
gcc/
* Makefile.in (PLUGIN_HEADERS): Add include/xtensa-dynconfig.h.
* config.gcc (xtensa*-*-*): Add xtensa-config.o to extra_objs.
* gcc/config/xtensa/t-xtensa (xtensa-config.o): New rule.
* gcc/config/xtensa/xtensa-config.c: New file.
* gcc/config/xtensa/xtensa.h (xtensa-config.h): Replace #include
with xtensa-dynconfig.h
(XCHAL_HAVE_MUL32_HIGH, XCHAL_HAVE_RELEASE_SYNC,
 XCHAL_HAVE_S32C1I, XCHAL_HAVE_THREADPTR,
 XCHAL_HAVE_FP_POSTINC): Drop definitions.

include/
* xtensa-dynconfig.h: New file.
---
 gcc/Makefile.in   |   2 +-
 gcc/config.gcc|   1 +
 gcc/config/xtensa/t-xtensa|   5 +
 gcc/config/xtensa/xtensa-config.c |  79 
 gcc/config/xtensa/xtensa.h|  17 +-
 include/xtensa-dynconfig.h| 373 ++
 6 files changed, 460 insertions(+), 17 deletions(-)
 create mode 100644 gcc/config/xtensa/xtensa-config.c
 create mode 100644 include/xtensa-dynconfig.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 2411671..f59a29f 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3433,7 +3433,7 @@ PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $(SYSTEM_H) 
coretypes.h $(TM_H) \
   tree-ssa-threadupdate.h inchash.h wide-int.h signop.h hash-map.h \
   hash-set.h dominance.h cfg.h cfgrtl.h cfganal.h cfgbuild.h cfgcleanup.h \
   lcm.h cfgloopmanip.h builtins.def chkp-builtins.def pass-instances.def \
-  params.list
+  params.list $(srcdir)/../include/xtensa-dynconfig.h
 
 # generate the 'build fragment' b-header-vars
 s-header-vars: Makefile
diff --git a/gcc/config.gcc b/gcc/config.gcc
index b8bb4d6..f65bb2e 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -489,6 +489,7 @@ tic6x-*-*)
;;
 xtensa*-*-*)
extra_options="${extra_options} fused-madd.opt"
+   extra_objs="xtensa-config.o"
;;
 tilegx*-*-*)
cpu_type=tilegx
diff --git a/gcc/config/xtensa/t-xtensa b/gcc/config/xtensa/t-xtensa
index f762873..28d3756 100644
--- a/gcc/config/xtensa/t-xtensa
+++ b/gcc/config/xtensa/t-xtensa
@@ -17,3 +17,8 @@
 # .
 
 $(out_object_file): gt-xtensa.h
+
+xtensa-config.o: $(srcdir)/config/xtensa/xtensa-config.c \
+  $(CONFIG_H) $(SYSTEM_H) $(srcdir)/../include/xtensa-dynconfig.h \
+  $(srcdir)/../include/xtensa-config.h
+   $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $<
diff --git a/gcc/config/xtensa/xtensa-config.c 
b/gcc/config/xtensa/xtensa-config.c
new file mode 100644
index 000..296caf4
--- /dev/null
+++ b/gcc/config/xtensa/xtensa-config.c
@@ -0,0 +1,79 @@
+#include 
+#include 
+#define XTENSA_CONFIG_DEFINITION
+#include "xtensa-config.h"
+#include "xtensa-dynconfig.h"
+
+static struct xtensa_config xtensa_defconfig = XTENSA_CONFIG_INITIALIZER;
+
+void *xtensa_load_config (const char *name ATTRIBUTE_UNUSED, void *def)
+{
+  static int init;
+#ifdef ENABLE_PLUGIN
+  static void *handle;
+  void *p;
+
+  if (!init)
+{
+  char *path = getenv ("XTENSA_GNU_CONFIG");
+
+  init = 1;
+  if (!path)
+   return def;
+  handle = dlopen (path, RTLD_LAZY);
+  if (!handle)
+   {
+ fprintf (stderr,
+  "XTENSA_GNU_CONFIG is defined but could not be loaded: %s\n",
+  dlerror ());
+ abort ();
+   }
+}
+  else if (!handle)
+{
+  return def;
+}
+
+  p = dlsym (handle, name);
+  if (!p)
+{
+  fprintf (stderr,
+  "XTENSA_GNU_CONFIG is loaded but symbol \"%s\" is not found: 
%s\n",
+  name, dlerror ());
+  abort ();
+}
+  return p;
+#else
+  if (!init)
+{
+  char *path = getenv ("XTENSA_GNU_CONFIG");
+
+  init = 1;
+  if (path)
+   {
+ fprintf (stderr,
+  "XTENSA_GNU_CONFIG is defined but plugin support is 
disabled\n");
+ abort ();
+   }
+}
+  return def;
+#endif
+}
+
+struct xtensa_config *xtensa_get_config (void)
+{
+  static struct xtensa_config *config;
+
+  if (!config)
+config = (struct xtensa_config *) xtensa_load_config ("xtensa_config",
+ _defconfig);
+
+  if (config->config_size < sizeof(struct xtensa_config))
+{
+  fprintf (stderr,
+  "Old or incompatible configuration is loaded: config_size = %ld, 
expected: