public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Proposed gcc plugin plugin API mk 2 (this time without camel case!)
@ 2012-04-02 17:22 David Malcolm
  2012-04-03 10:04 ` Richard Guenther
  2012-04-12 13:12 ` Ludovic Courtès
  0 siblings, 2 replies; 9+ messages in thread
From: David Malcolm @ 2012-04-02 17:22 UTC (permalink / raw)
  To: gcc

I wrote a script and ported my proposed API for GCC plugins from my
CamelCase naming convention to an underscore_based_convention (and
manually fixed up things in a few places also).

The result compiles and passes the full test suite for the Python
plugin; that said, I'm still breaking the encapsulation in quite a few
places (hey, this is an experimental prototype).

You can see the latest version of it within the "proposed-plugin-api"
branch of the Python plugin here:
http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=shortlog;h=refs/heads/proposed-plugin-api

within the "proposed-plugin-api" subdirectory.

How is this looking?

If this landed e.g. in GCC 4.8, would this be usable by other plugins?

Hope this is helpful
Dave

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Proposed gcc plugin plugin API mk 2 (this time without camel case!)
  2012-04-02 17:22 Proposed gcc plugin plugin API mk 2 (this time without camel case!) David Malcolm
@ 2012-04-03 10:04 ` Richard Guenther
  2012-04-03 13:23   ` Richard Guenther
  2012-04-03 15:15   ` David Malcolm
  2012-04-12 13:12 ` Ludovic Courtès
  1 sibling, 2 replies; 9+ messages in thread
From: Richard Guenther @ 2012-04-03 10:04 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc

On Mon, Apr 2, 2012 at 7:21 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> I wrote a script and ported my proposed API for GCC plugins from my
> CamelCase naming convention to an underscore_based_convention (and
> manually fixed up things in a few places also).
>
> The result compiles and passes the full test suite for the Python
> plugin; that said, I'm still breaking the encapsulation in quite a few
> places (hey, this is an experimental prototype).
>
> You can see the latest version of it within the "proposed-plugin-api"
> branch of the Python plugin here:
> http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=shortlog;h=refs/heads/proposed-plugin-api
>
> within the "proposed-plugin-api" subdirectory.

Hmm, how do I get it?  I did

git clone http://git.fedorahosted.org/git/proposed-plugin-api

but there is nothing in gcc-python-plugin/.  And

git checkout proposed-plugin-api

says I'm already there ...?

Richard.

> How is this looking?
>
> If this landed e.g. in GCC 4.8, would this be usable by other plugins?
>
> Hope this is helpful
> Dave
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Proposed gcc plugin plugin API mk 2 (this time without camel case!)
  2012-04-03 10:04 ` Richard Guenther
@ 2012-04-03 13:23   ` Richard Guenther
  2012-04-03 16:03     ` David Malcolm
  2012-04-03 15:15   ` David Malcolm
  1 sibling, 1 reply; 9+ messages in thread
From: Richard Guenther @ 2012-04-03 13:23 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc

On Tue, Apr 3, 2012 at 12:03 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Mon, Apr 2, 2012 at 7:21 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>> I wrote a script and ported my proposed API for GCC plugins from my
>> CamelCase naming convention to an underscore_based_convention (and
>> manually fixed up things in a few places also).
>>
>> The result compiles and passes the full test suite for the Python
>> plugin; that said, I'm still breaking the encapsulation in quite a few
>> places (hey, this is an experimental prototype).
>>
>> You can see the latest version of it within the "proposed-plugin-api"
>> branch of the Python plugin here:
>> http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=shortlog;h=refs/heads/proposed-plugin-api
>>
>> within the "proposed-plugin-api" subdirectory.
>
> Hmm, how do I get it?  I did
>
> git clone http://git.fedorahosted.org/git/proposed-plugin-api
>
> but there is nothing in gcc-python-plugin/.  And
>
> git checkout proposed-plugin-api
>
> says I'm already there ...?

Meanwhile the directory magically appeared (heh ...).

Overall it looks good - but it seems to leak GCC headers into the
plugin API (via gcc-plugin.h and input.h inclusion).  Thus, it
lacks separating the plugin API headers from the plugin API implementation
headers?  Or maybe I'm not looking at the right place.
I also dislike the use of GCC_PUBLIC_API, etc. - everything in
the plugin API headers should be obviously public - and the implementation
detail should be an implementation detail that should not need to care.

>> If this landed e.g. in GCC 4.8, would this be usable by other plugins?

For sure.  I'd say get the copyright stuff sorted out and start pushing things
into the main GCC repository - where the obvious starting point is to
get the makefile, configure and install parts correct.

I'd concentrate on providing enough API for introspection, like simple
things, counting basic-blocks, counting calls, etc.  I'm not sure we
want to expose gcc_gimple_walk_tree or gcc_gimple_print (or
the gcc_printers for a start) - the latter would something that the
python plugin
would provide, resorting to introspecting the stmt itself.

I also wonder about gcc_gimple_phi_upcast - why would you specialize
PHI nodes but not any others?  I'd have exposed gcc_gimple_code.
In general the plugin API should provide exactly one (and the most flexible)
way to do a thing (thus, not have gcc_gimple_assign_single_p) and
the high-level consumers like the python plugin should provide
nice-to-use interfaces.

Thanks,
Richard.

>> Hope this is helpful
>> Dave
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Proposed gcc plugin plugin API mk 2 (this time without camel case!)
  2012-04-03 10:04 ` Richard Guenther
  2012-04-03 13:23   ` Richard Guenther
@ 2012-04-03 15:15   ` David Malcolm
  1 sibling, 0 replies; 9+ messages in thread
From: David Malcolm @ 2012-04-03 15:15 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc

On Tue, 2012-04-03 at 12:03 +0200, Richard Guenther wrote:
> On Mon, Apr 2, 2012 at 7:21 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> > I wrote a script and ported my proposed API for GCC plugins from my
> > CamelCase naming convention to an underscore_based_convention (and
> > manually fixed up things in a few places also).
> >
> > The result compiles and passes the full test suite for the Python
> > plugin; that said, I'm still breaking the encapsulation in quite a few
> > places (hey, this is an experimental prototype).
> >
> > You can see the latest version of it within the "proposed-plugin-api"
> > branch of the Python plugin here:
> > http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=shortlog;h=refs/heads/proposed-plugin-api
> >
> > within the "proposed-plugin-api" subdirectory.
> 
> Hmm, how do I get it?  I did
> 
> git clone http://git.fedorahosted.org/git/proposed-plugin-api
> 
> but there is nothing in gcc-python-plugin/.  And
> 
> git checkout proposed-plugin-api
> 
> says I'm already there ...?
I'm sorry that this was unclear.

To checkout the source, on the "proposed-plugin-api" branch:

  $ git clone git://git.fedorahosted.org/gcc-python-plugin.git -b proposed-plugin-api
  Cloning into gcc-python-plugin...
  remote: Counting objects: 8375, done.
    [...snip...]
  Resolving deltas: 100% (5774/5774), done.

Go into the new working copy:
  $ cd gcc-python-plugin/

Verify that you're on the experimental branch:
  $ git branch
  * proposed-plugin-api

The code in the root directory is specific to my Python plugin.  The
proposed generic plugin API is in a subdirectory which confusingly has
the same name as the branch (sorry):
  $ cd proposed-plugin-api

(If you're interested in the python plugin, detailed instructions on
building from source can be seen at [1])

Sorry about the confusion
Dave

[1]
http://gcc-python-plugin.readthedocs.org/en/latest/basics.html#building-the-plugin-from-source


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Proposed gcc plugin plugin API mk 2 (this time without camel case!)
  2012-04-03 13:23   ` Richard Guenther
@ 2012-04-03 16:03     ` David Malcolm
  2012-04-03 18:15       ` Romain Geissler
  2012-04-04  8:35       ` Richard Guenther
  0 siblings, 2 replies; 9+ messages in thread
From: David Malcolm @ 2012-04-03 16:03 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc

On Tue, 2012-04-03 at 15:23 +0200, Richard Guenther wrote:
> On Tue, Apr 3, 2012 at 12:03 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
> > On Mon, Apr 2, 2012 at 7:21 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> >> I wrote a script and ported my proposed API for GCC plugins from my
> >> CamelCase naming convention to an underscore_based_convention (and
> >> manually fixed up things in a few places also).
> >>
> >> The result compiles and passes the full test suite for the Python
> >> plugin; that said, I'm still breaking the encapsulation in quite a few
> >> places (hey, this is an experimental prototype).
> >>
> >> You can see the latest version of it within the "proposed-plugin-api"
> >> branch of the Python plugin here:
> >> http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=shortlog;h=refs/heads/proposed-plugin-api
> >>
> >> within the "proposed-plugin-api" subdirectory.
> >
> > Hmm, how do I get it?  I did
> >
> > git clone http://git.fedorahosted.org/git/proposed-plugin-api
> >
> > but there is nothing in gcc-python-plugin/.  And
> >
> > git checkout proposed-plugin-api
> >
> > says I'm already there ...?
> 
> Meanwhile the directory magically appeared (heh ...).

[The ways of git are something of a mystery to me: 95% of the time it's
the best revision control system I've ever used, but 5% of the time the
most obtuse]

> Overall it looks good
Thanks for taking a look.

>  - but it seems to leak GCC headers into the
> plugin API (via gcc-plugin.h and input.h inclusion).  Thus, it
> lacks separating the plugin API headers from the plugin API implementation
> headers?  
That's true.  The big information "leak" happens inside
gcc-semiprivate-types.h, which defines the various structs that act like
pointers, each with a decl like this:

struct gcc_something {
   some_private_gcc_pointer_type inner;
};

It would be possible to make this more opaque like this:

struct gcc_something {
   struct some_private_gcc_struct *inner;
};

given that you then don't need a full definition of that inner struct
visible to users.  Though location_t is leaked, and in this approach,
there isn't a way around that, I think.


> Or maybe I'm not looking at the right place.
> I also dislike the use of GCC_PUBLIC_API, etc. - everything in
> the plugin API headers should be obviously public - and the implementation
> detail should be an implementation detail that should not need to care.

I added that macro based on the example of other plugin/embedding
systems I've seen (e.g. Python), where it's handy to make that macro be
non-trivial on some platforms.  See e.g. CPython's pyport.h:
 http://hg.python.org/cpython/file/9599f091faa6/Include/pyport.h
where the macros PyAPI_FUNC and PyAPI_DATA have a 44-line definition,
handling various different compatibility cases.

For example, GCC_PRIVATE_API could have some magic linker flag that lets
us automatically strip out those symbols so that they're not visible
externally after the compiler has been linked.

> >> If this landed e.g. in GCC 4.8, would this be usable by other plugins?
> 
> For sure.  I'd say get the copyright stuff sorted out and start pushing things
> into the main GCC repository - where the obvious starting point is to
> get the makefile, configure and install parts correct.
> 
> I'd concentrate on providing enough API for introspection, like simple
> things, counting basic-blocks, counting calls, etc.  I'm not sure we
> want to expose gcc_gimple_walk_tree or gcc_gimple_print (or
> the gcc_printers for a start) - the latter would something that the
> python plugin
> would provide, resorting to introspecting the stmt itself.
FWIW the Python plugin already heavily uses gcc's pretty-printer code,
so that *is* something I'd want to wrap (but it's fairly easy to do so).

> I also wonder about gcc_gimple_phi_upcast - why would you specialize
> PHI nodes but not any others?  I'd have exposed gcc_gimple_code.
> In general the plugin API should provide exactly one (and the most flexible)
> way to do a thing (thus, not have gcc_gimple_assign_single_p) and
> the high-level consumers like the python plugin should provide
> nice-to-use interfaces.

This touches on the biggest thing that's missing in the API: the
multitude of tree types, gimple statement types, and rtl types.  All I
added were the "base classes", and gimple-phi is the only instance so
far in the API of a subclass.

I would like the API to expose these (my plugin uses them to good
effect), but doing it in C is likely to be messy: lots of upcasting and
downcasting functions.

An alternative approach might be to bite the bullet and go to C++; maybe
something like this (caveat: I last did any C++ about a decade ago, no
idea if the following compiles):

// Everything exposed in the public headers are pure virtual functions
// leading to abstract classes: no public data
namespace gcc {

   // Abstract base class for objects managed by GCC's garbage-collector
   class gcmanaged {
   public:
      virtual void mark_in_use() const = 0;
   };

   class location : public gcmanaged {
       // it isn't gcmanaged yet, but might become so
   public:
      virtual std::string get_filename() const = 0;
      virtual int get_line() const = 0;
      virtual int get_column() const = 0;
   };

   namespace gimple {
       // enum to assist in RTTI by non-C++ code:
       enum code {
           INVALID_GIMPLE_CODE = -1,
	   GIMPLE_NOP = 0,
	   GIMPLE_ASSIGNMENT,
	   GIMPLE_SWITCH,
	   //etc; don't have to expose all of the regular gimple codes.
           NUM_CODES,
       };

       class statement : public gcmanaged {
       public:
	   virtual location *get_location() const = 0;
	   virtual enum code get_code() const = 0;
       };

       class nop : public statement {
       };

       class phi : public statement {
       public:
           virtual tree::ssaname* get_lhs() const = 0;
           virtual std::list<tree::expr*, cfg::edge*> get_rhs() const = 0;
           // or whatever
       };
   };

   namespace tree {
       class declaration : public gcmanaged {
       public:
	   virtual std::string get_name() const = 0;
	   virtual location*   get_location() const = 0;
       };

       class funcdecl :: public declaration {
       public:
	   virtual function* get_function() const = 0;
	   virtual std::list<parmdecl*> get_arguments() const = 0;
	   virtual resultdecl* get_result() const = 0;
           // etc: accessor functions
       };

       class type : public gcmanaged {
       // etc...
       };

       class integertype :: public type {
       // etc...
       };

       // does there actually need to be a "tree" base class?  I think
       // the tcc_codes express the base classes, at a first approximation
       // could even lose the "tree" concept altogether
   };
};

[though I'd greatly prefer to capitalize the classnames (sigh) :) ]

The implementation would have entirely separate (private) headers, with
something like:

class location_impl : public gcc::location {
private:
   location_t loc;

public:
    // ctor:
    location_impl(location_t loc) : loc(loc) {} 

    // implement the public API:
    void mark_in_use() const {
         // empty: location_t currently isn't gc managed
    }

    std::string get_filename() const;
    int get_line() const;
    int get_column() const;
};

class phi_impl : public gcc::gimple::phi {
private:
    gimple stmt; // of correct subtype

public:
    // gcmanaged:
    void mark_in_use() const;

    // ctor:
    phi(gimple stmt);

    // accessors:
    location *get_location() const;
    enum code get_code() const;

    tree::ssaname *get_lhs() const;
    std::list<tree::expr*, cfg::edge*> get_rhs() const;
};

That way the implementation is *entirely* hidden, with entirely separate
headers (though during prototyping I'd need an escape hatch).  But it
does mean that the API is handing off pointers-to-pointers, rather than
just pointers (allocating lots of tiny wrapper objects, like the ones
above, so that the caller doesn't know the size, and so there can be a
vtable).  

Does this sound sane?

Long-term, perhaps these impl classes could perhaps *become* the
implementations - everything's hidden.

Thanks again for looking at this
Dave

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Proposed gcc plugin plugin API mk 2 (this time without camel case!)
  2012-04-03 16:03     ` David Malcolm
@ 2012-04-03 18:15       ` Romain Geissler
  2012-04-04  8:27         ` Richard Guenther
  2012-04-04  8:35       ` Richard Guenther
  1 sibling, 1 reply; 9+ messages in thread
From: Romain Geissler @ 2012-04-03 18:15 UTC (permalink / raw)
  To: David Malcolm; +Cc: Richard Guenther, gcc


Le 3 avr. 2012 à 18:02, David Malcolm a écrit :

> On Tue, 2012-04-03 at 15:23 +0200, Richard Guenther wrote:
>> On Tue, Apr 3, 2012 at 12:03 PM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Mon, Apr 2, 2012 at 7:21 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>>>> I wrote a script and ported my proposed API for GCC plugins from my
>>>> CamelCase naming convention to an underscore_based_convention (and
>>>> manually fixed up things in a few places also).
>>>> 
>>>> The result compiles and passes the full test suite for the Python
>>>> plugin; that said, I'm still breaking the encapsulation in quite a few
>>>> places (hey, this is an experimental prototype).
>>>> 
>>>> You can see the latest version of it within the "proposed-plugin-api"
>>>> branch of the Python plugin here:
>>>> http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=shortlog;h=refs/heads/proposed-plugin-api
>>>> 
>>>> within the "proposed-plugin-api" subdirectory.
>>> 
>>> Hmm, how do I get it?  I did
>>> 
>>> git clone http://git.fedorahosted.org/git/proposed-plugin-api
>>> 
>>> but there is nothing in gcc-python-plugin/.  And
>>> 
>>> git checkout proposed-plugin-api
>>> 
>>> says I'm already there ...?
>> 
>> Meanwhile the directory magically appeared (heh ...).
> 
> [The ways of git are something of a mystery to me: 95% of the time it's
> the best revision control system I've ever used, but 5% of the time the
> most obtuse]
> 
>> Overall it looks good
> Thanks for taking a look.
> 
>> - but it seems to leak GCC headers into the
>> plugin API (via gcc-plugin.h and input.h inclusion).  Thus, it
>> lacks separating the plugin API headers from the plugin API implementation
>> headers?  
> That's true.  The big information "leak" happens inside
> gcc-semiprivate-types.h, which defines the various structs that act like
> pointers, each with a decl like this:
> 
> struct gcc_something {
>   some_private_gcc_pointer_type inner;
> };
> 
> It would be possible to make this more opaque like this:
> 
> struct gcc_something {
>   struct some_private_gcc_struct *inner;
> };
> 
> given that you then don't need a full definition of that inner struct
> visible to users.  Though location_t is leaked, and in this approach,
> there isn't a way around that, I think.

Well i think we you should define a public type like this :

typedef struct some_private_gcc_struct *gcc_something;

extern some_type retrieve_some_value(gcc_something);

Also, nothing should be noted public or private : all definitions
that will appear in a header installed in
$(gcc -print-file-name=plugin)/include will be public by definition.

Any additional header that would be needed to implement the
API should be kept separate (like the actual *.c implementing it)
and placed in the gcc/ directory in the trunk (or better something
like gcc/plugin-impl/ to start being modular). Any definition defined
in those additional headers are private.

Thus, you should define two sets of headers files (public and private ones),
plus body c files, and import only public header files from public header files.

Do you have any plan on starting integrating it to the trunk (or at least on an
new branch on the official gcc repository) soon, like suggested by Richard ?
I might help setting up the configure/makefile and later writing some wrappers.
(although i don't have write permission).

Cheers

Romain Geissler

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Proposed gcc plugin plugin API mk 2 (this time without camel case!)
  2012-04-03 18:15       ` Romain Geissler
@ 2012-04-04  8:27         ` Richard Guenther
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Guenther @ 2012-04-04  8:27 UTC (permalink / raw)
  To: Romain Geissler; +Cc: David Malcolm, gcc

On Tue, Apr 3, 2012 at 8:15 PM, Romain Geissler
<romain.geissler@gmail.com> wrote:
>
> Le 3 avr. 2012 à 18:02, David Malcolm a écrit :
>
>> On Tue, 2012-04-03 at 15:23 +0200, Richard Guenther wrote:
>>> On Tue, Apr 3, 2012 at 12:03 PM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Mon, Apr 2, 2012 at 7:21 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>>>>> I wrote a script and ported my proposed API for GCC plugins from my
>>>>> CamelCase naming convention to an underscore_based_convention (and
>>>>> manually fixed up things in a few places also).
>>>>>
>>>>> The result compiles and passes the full test suite for the Python
>>>>> plugin; that said, I'm still breaking the encapsulation in quite a few
>>>>> places (hey, this is an experimental prototype).
>>>>>
>>>>> You can see the latest version of it within the "proposed-plugin-api"
>>>>> branch of the Python plugin here:
>>>>> http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=shortlog;h=refs/heads/proposed-plugin-api
>>>>>
>>>>> within the "proposed-plugin-api" subdirectory.
>>>>
>>>> Hmm, how do I get it?  I did
>>>>
>>>> git clone http://git.fedorahosted.org/git/proposed-plugin-api
>>>>
>>>> but there is nothing in gcc-python-plugin/.  And
>>>>
>>>> git checkout proposed-plugin-api
>>>>
>>>> says I'm already there ...?
>>>
>>> Meanwhile the directory magically appeared (heh ...).
>>
>> [The ways of git are something of a mystery to me: 95% of the time it's
>> the best revision control system I've ever used, but 5% of the time the
>> most obtuse]
>>
>>> Overall it looks good
>> Thanks for taking a look.
>>
>>> - but it seems to leak GCC headers into the
>>> plugin API (via gcc-plugin.h and input.h inclusion).  Thus, it
>>> lacks separating the plugin API headers from the plugin API implementation
>>> headers?
>> That's true.  The big information "leak" happens inside
>> gcc-semiprivate-types.h, which defines the various structs that act like
>> pointers, each with a decl like this:
>>
>> struct gcc_something {
>>   some_private_gcc_pointer_type inner;
>> };
>>
>> It would be possible to make this more opaque like this:
>>
>> struct gcc_something {
>>   struct some_private_gcc_struct *inner;
>> };
>>
>> given that you then don't need a full definition of that inner struct
>> visible to users.  Though location_t is leaked, and in this approach,
>> there isn't a way around that, I think.
>
> Well i think we you should define a public type like this :
>
> typedef struct some_private_gcc_struct *gcc_something;
>
> extern some_type retrieve_some_value(gcc_something);

Indeed.  I'd go one step further and do

typedef struct gcc_something_s *gcc_something;

and in the implementations only expose struct some_private_gcc_struct
by means of casting the pointer.  For the public API gcc_something
is a pointer to an opaque thing, never dereferenced.

> Also, nothing should be noted public or private : all definitions
> that will appear in a header installed in
> $(gcc -print-file-name=plugin)/include will be public by definition.

Right.

Richard.

> Any additional header that would be needed to implement the
> API should be kept separate (like the actual *.c implementing it)
> and placed in the gcc/ directory in the trunk (or better something
> like gcc/plugin-impl/ to start being modular). Any definition defined
> in those additional headers are private.
>
> Thus, you should define two sets of headers files (public and private ones),
> plus body c files, and import only public header files from public header files.
>
> Do you have any plan on starting integrating it to the trunk (or at least on an
> new branch on the official gcc repository) soon, like suggested by Richard ?
> I might help setting up the configure/makefile and later writing some wrappers.
> (although i don't have write permission).


> Cheers
>
> Romain Geissler

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Proposed gcc plugin plugin API mk 2 (this time without camel case!)
  2012-04-03 16:03     ` David Malcolm
  2012-04-03 18:15       ` Romain Geissler
@ 2012-04-04  8:35       ` Richard Guenther
  1 sibling, 0 replies; 9+ messages in thread
From: Richard Guenther @ 2012-04-04  8:35 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc

On Tue, Apr 3, 2012 at 6:02 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Tue, 2012-04-03 at 15:23 +0200, Richard Guenther wrote:
>> On Tue, Apr 3, 2012 at 12:03 PM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>> > On Mon, Apr 2, 2012 at 7:21 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>> >> I wrote a script and ported my proposed API for GCC plugins from my
>> >> CamelCase naming convention to an underscore_based_convention (and
>> >> manually fixed up things in a few places also).
>> >>
>> >> The result compiles and passes the full test suite for the Python
>> >> plugin; that said, I'm still breaking the encapsulation in quite a few
>> >> places (hey, this is an experimental prototype).
>> >>
>> >> You can see the latest version of it within the "proposed-plugin-api"
>> >> branch of the Python plugin here:
>> >> http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=shortlog;h=refs/heads/proposed-plugin-api
>> >>
>> >> within the "proposed-plugin-api" subdirectory.
>> >
>> > Hmm, how do I get it?  I did
>> >
>> > git clone http://git.fedorahosted.org/git/proposed-plugin-api
>> >
>> > but there is nothing in gcc-python-plugin/.  And
>> >
>> > git checkout proposed-plugin-api
>> >
>> > says I'm already there ...?
>>
>> Meanwhile the directory magically appeared (heh ...).
>
> [The ways of git are something of a mystery to me: 95% of the time it's
> the best revision control system I've ever used, but 5% of the time the
> most obtuse]
>
>> Overall it looks good
> Thanks for taking a look.
>
>>  - but it seems to leak GCC headers into the
>> plugin API (via gcc-plugin.h and input.h inclusion).  Thus, it
>> lacks separating the plugin API headers from the plugin API implementation
>> headers?
> That's true.  The big information "leak" happens inside
> gcc-semiprivate-types.h, which defines the various structs that act like
> pointers, each with a decl like this:
>
> struct gcc_something {
>   some_private_gcc_pointer_type inner;
> };
>
> It would be possible to make this more opaque like this:
>
> struct gcc_something {
>   struct some_private_gcc_struct *inner;
> };
>
> given that you then don't need a full definition of that inner struct
> visible to users.  Though location_t is leaked, and in this approach,
> there isn't a way around that, I think.

See the reply by Roman.

>
>> Or maybe I'm not looking at the right place.
>> I also dislike the use of GCC_PUBLIC_API, etc. - everything in
>> the plugin API headers should be obviously public - and the implementation
>> detail should be an implementation detail that should not need to care.
>
> I added that macro based on the example of other plugin/embedding
> systems I've seen (e.g. Python), where it's handy to make that macro be
> non-trivial on some platforms.  See e.g. CPython's pyport.h:
>  http://hg.python.org/cpython/file/9599f091faa6/Include/pyport.h
> where the macros PyAPI_FUNC and PyAPI_DATA have a 44-line definition,
> handling various different compatibility cases.
>
> For example, GCC_PRIVATE_API could have some magic linker flag that lets
> us automatically strip out those symbols so that they're not visible
> externally after the compiler has been linked.

That's for the implementation side - the public API side by definition has
only declarations considered public.

>> >> If this landed e.g. in GCC 4.8, would this be usable by other plugins?
>>
>> For sure.  I'd say get the copyright stuff sorted out and start pushing things
>> into the main GCC repository - where the obvious starting point is to
>> get the makefile, configure and install parts correct.
>>
>> I'd concentrate on providing enough API for introspection, like simple
>> things, counting basic-blocks, counting calls, etc.  I'm not sure we
>> want to expose gcc_gimple_walk_tree or gcc_gimple_print (or
>> the gcc_printers for a start) - the latter would something that the
>> python plugin
>> would provide, resorting to introspecting the stmt itself.
> FWIW the Python plugin already heavily uses gcc's pretty-printer code,
> so that *is* something I'd want to wrap (but it's fairly easy to do so).

Ok, fair enough.

>> I also wonder about gcc_gimple_phi_upcast - why would you specialize
>> PHI nodes but not any others?  I'd have exposed gcc_gimple_code.
>> In general the plugin API should provide exactly one (and the most flexible)
>> way to do a thing (thus, not have gcc_gimple_assign_single_p) and
>> the high-level consumers like the python plugin should provide
>> nice-to-use interfaces.
>
> This touches on the biggest thing that's missing in the API: the
> multitude of tree types, gimple statement types, and rtl types.  All I
> added were the "base classes", and gimple-phi is the only instance so
> far in the API of a subclass.

Well, one way is surely to view it as sort-of "classes", the other way is
to view it as tagged structures which you can simply expose.  Of course
if there will be a day where somebody converts all of gimple and tree
to a C++ class hierarchy the class way would be a better representation.

> I would like the API to expose these (my plugin uses them to good
> effect), but doing it in C is likely to be messy: lots of upcasting and
> downcasting functions.

I'd not do up/down-casting.  I'd at most expose a RTTI-like interface,
gcc_gimple_is_phi, etc..  Using an opaque "base" pointer in all
functions should be ok (ok, you will miss some type safety - but so
does GCC internally).

> An alternative approach might be to bite the bullet and go to C++; maybe
> something like this (caveat: I last did any C++ about a decade ago, no
> idea if the following compiles):
>
> // Everything exposed in the public headers are pure virtual functions
> // leading to abstract classes: no public data
> namespace gcc {
>
>   // Abstract base class for objects managed by GCC's garbage-collector
>   class gcmanaged {
>   public:
>      virtual void mark_in_use() const = 0;
>   };
>
>   class location : public gcmanaged {
>       // it isn't gcmanaged yet, but might become so
>   public:
>      virtual std::string get_filename() const = 0;
>      virtual int get_line() const = 0;
>      virtual int get_column() const = 0;
>   };
>
>   namespace gimple {
>       // enum to assist in RTTI by non-C++ code:
>       enum code {
>           INVALID_GIMPLE_CODE = -1,
>           GIMPLE_NOP = 0,
>           GIMPLE_ASSIGNMENT,
>           GIMPLE_SWITCH,
>           //etc; don't have to expose all of the regular gimple codes.
>           NUM_CODES,
>       };
>
>       class statement : public gcmanaged {
>       public:
>           virtual location *get_location() const = 0;
>           virtual enum code get_code() const = 0;
>       };
>
>       class nop : public statement {
>       };
>
>       class phi : public statement {
>       public:
>           virtual tree::ssaname* get_lhs() const = 0;
>           virtual std::list<tree::expr*, cfg::edge*> get_rhs() const = 0;
>           // or whatever
>       };
>   };
>
>   namespace tree {
>       class declaration : public gcmanaged {
>       public:
>           virtual std::string get_name() const = 0;
>           virtual location*   get_location() const = 0;
>       };
>
>       class funcdecl :: public declaration {
>       public:
>           virtual function* get_function() const = 0;
>           virtual std::list<parmdecl*> get_arguments() const = 0;
>           virtual resultdecl* get_result() const = 0;
>           // etc: accessor functions
>       };
>
>       class type : public gcmanaged {
>       // etc...
>       };
>
>       class integertype :: public type {
>       // etc...
>       };
>
>       // does there actually need to be a "tree" base class?  I think
>       // the tcc_codes express the base classes, at a first approximation
>       // could even lose the "tree" concept altogether
>   };
> };

But that of course requires real objects to mediate between the plugin API
and GCC internals - something I'd try to avoid hard.  Objects should be
identified by pointers to the original internal structures - that also makes
it simple to eventually add a garbage collector friendly reference taking
mechanism (by simply having a new internal GC root being a pointer-map,
ptr:refcount and a plugin API that does gcc_push_ref () / gcc_pop_ref ()).

> [though I'd greatly prefer to capitalize the classnames (sigh) :) ]
>
> The implementation would have entirely separate (private) headers, with
> something like:
>
> class location_impl : public gcc::location {
> private:
>   location_t loc;
>
> public:
>    // ctor:
>    location_impl(location_t loc) : loc(loc) {}
>
>    // implement the public API:
>    void mark_in_use() const {
>         // empty: location_t currently isn't gc managed
>    }
>
>    std::string get_filename() const;
>    int get_line() const;
>    int get_column() const;
> };
>
> class phi_impl : public gcc::gimple::phi {
> private:
>    gimple stmt; // of correct subtype
>
> public:
>    // gcmanaged:
>    void mark_in_use() const;
>
>    // ctor:
>    phi(gimple stmt);
>
>    // accessors:
>    location *get_location() const;
>    enum code get_code() const;
>
>    tree::ssaname *get_lhs() const;
>    std::list<tree::expr*, cfg::edge*> get_rhs() const;
> };
>
> That way the implementation is *entirely* hidden, with entirely separate
> headers (though during prototyping I'd need an escape hatch).  But it
> does mean that the API is handing off pointers-to-pointers, rather than
> just pointers (allocating lots of tiny wrapper objects, like the ones
> above, so that the caller doesn't know the size, and so there can be a
> vtable).
>
> Does this sound sane?

No, requiring extra storage allocation will greatly complicate API use.

Richard.

> Long-term, perhaps these impl classes could perhaps *become* the
> implementations - everything's hidden.
>
> Thanks again for looking at this
> Dave
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Proposed gcc plugin plugin API mk 2 (this time without camel case!)
  2012-04-02 17:22 Proposed gcc plugin plugin API mk 2 (this time without camel case!) David Malcolm
  2012-04-03 10:04 ` Richard Guenther
@ 2012-04-12 13:12 ` Ludovic Courtès
  1 sibling, 0 replies; 9+ messages in thread
From: Ludovic Courtès @ 2012-04-12 13:12 UTC (permalink / raw)
  To: gcc

Hi,

(Sorry for the delay.)

I suppose the proposed API doesn’t cover all the needs of your Python
bindings and their applications, does it?  How do you plan to export the
GIMPLE and tree.h APIs?

Regarding iterators, there are things like:

  GCC_IMPLEMENT_PUBLIC_API(bool)
  gcc_cfg_for_each_block(gcc_cfg cfg,
                       bool (*cb)(gcc_cfg_block block, void *user_data),
                       void *user_data)
  {
      int i;

      for (i = 0; i < cfg.inner->x_n_basic_blocks; i++) {

Instead of the callback API, could the details of the containers be
exposed a little more, like:

  /* Return the number of basic blocks in CFG.  */
  unsigned int gcc_cfg_block_count (gcc_cfg cfg);

  /* Return a pointer to the array of basic blocks in CFG.  */
  gcc_cfg_block *gcc_cfg_blocks (gcc_cfg cfg);

That would allow higher-level language bindings to provide idiomatic
ways to iterate over basic blocks, such as (lazy) list iterators.

WDYT?

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-04-12 13:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-02 17:22 Proposed gcc plugin plugin API mk 2 (this time without camel case!) David Malcolm
2012-04-03 10:04 ` Richard Guenther
2012-04-03 13:23   ` Richard Guenther
2012-04-03 16:03     ` David Malcolm
2012-04-03 18:15       ` Romain Geissler
2012-04-04  8:27         ` Richard Guenther
2012-04-04  8:35       ` Richard Guenther
2012-04-03 15:15   ` David Malcolm
2012-04-12 13:12 ` Ludovic Courtès

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).