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]

[RFC] Compile-time gimple-checking (again)


Back in March I posted an 89-patch kit to expand and make use of the
gimple statement class hierarchy to move much of the  type-checking of
statement accessors to be at compile-time rather than run-time:
  https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01148.html

I'd like to get these patches into trunk in some form before stage 1
closes.

I'll attempt to summarize the earlier discussion about these patches;
please forgive me if I'm mischaracterizing things.

There was some discussion about what the resulting classes and API
should look like.

Jeff reviewed the patches and approved them (modulo some issues that
I've fixed), conditional on resolving the API design issues that arose
in the discussion - and on holding off until 4.9.1 was released.

Richi wanted me to change "gimple" to be the base class, rather than
being a typedef of a *pointer* to the base class:
   https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01520.html
thus avoiding numerous typedefs for all of the subclasses, in their
const and non-const variants. i.e. the "pointerness" of the type becomes
explicit; everywhere we currently have a:
  gimple stmt;
that's implicitly a ptr, we would instead have:
  gimple *stmt;
making the pointer explicit.

After some discussion about whether we wanted to keep the "gimple_"
prefix for the various subclasses , I posted an email with various ideas
as to what the API could look like:
  * Status quo
  * The April 2014 patch series (with indirect use of is-a.h)
  * Direct use of is-a.h, retaining typedefs of pointers
  * Explicit pointers, rather than typedefs
  * Implicit naming
  * Namespaces (explicit)
  * Namespaces (implicit)
  * C++ references (without namespaces)
  * C++ references (with implicit namespaces)
See https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01562.html for the
full examples.

There was a followup discussion about whether we should convert the
accessors to be *methods* rather than functions, but Richi felt that was
a step too far at this time:
https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01824.html

Jeff asked Richi (in
https://gcc.gnu.org/ml/gcc-patches/2014-04/msg02082.html ):
>> Anyway, gazillion new typedefs are ugly :/  (typedefs are ugly)
> Yea, can't argue with that. However, do we want to ask David to fix up
> the gimple vs gimple * vs const gimple * vs const gimple & as a
> prerequisite for this patchset or are you comfortable going forward
> with the patchset, then researching if there's a cleaner way to handle
> the const/typedef issues?

Richi responded (in
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00064.html):
> Well, I'd like to see both and one affects the other. Doing the
> const correctness thing first seems more natural to me.
> Of course both need to wait for 4.9.1.

I posted a possible renaming of the gimple subclasses as:
  https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00128.html
incorporating a mass-renaming, and changing the "pointerness" of the
gimple type to be explicit rather than implicit.

After some discussion about what the types should be named Jakub
suggested a simple "g" prefix:
  https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00248.html

giving a class hierarchy like this:
  https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00346.html

Jeff agreed:
  https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00347.html

Richi agreed:
  https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00349.html

(did I correctly summarize things?)

I first attempted to implement this by doing the "pointerness"
conversion first, to avoid lots of const_ typedefs, then updating the
other patches. i.e. a big autogenerated patch, followed by 89
handwritten patches.

I only managed about 1/3 of the kit, and what I had bitrotted very
quickly.
Based on that, I don't think it's workable to do the big automated
gimple -> gimple * conversion upfront.

I think two workable approaches are:
(i) save any big automated conversions until last
(ii) do an automated conversion on the .patch files themselves

I've updated the patch series; they can be seen at:
https://dmalcolm.fedorapeople.org/gcc/patch-backups/gimple-classes/v11-patches/

This contains the patches from before.

I've rebased them against r216157 from Monday (2014-10-13) aka
fc222f445c6108418196a1b48703d350f3c3d45a.

This required numerous essentially mechanical changes to the patches
e.g. for the big reworking of cgraph functions to be methods.  I've been
working on the assumption that these various changes aren't going to
require a re-review.

I also removed the unloved as_a/dyn_cast methods from the gimple base
class in favor of as_a<>/dyn_cast<> functions from is-a.h

I've successfully bootstrapped and regtested the cumulative effect of
the patchkit on Fedora 20 x86_64.

Some questions:

Are people still happy with the proposed naming from:
  https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00346.html
(incorporating the "pointerness" change of gimple to require "gimple
*").

How should I proceed?  The proposal is going to require one or more
auto-generated "megapatches", and such a patch is going to bitrot
immediately... and break everyone else's patches and branches that touch
the middle-end if/when it goes in (the "gimple" to "gimple *" change is
the issue here).

I'm thinking that I could attempt to put all of the handwritten stuff
onto a (git) branch, or possibly put the .patch files though a renaming
script to partially automated things first e.g. by updating the patches
to use explicit pointers for *subclasses* as the patchkit introduces
uses of subclasses.

Then the big "gimple" to "gimple *" conversion could be done mostly
separately, once the rest of the branch was ready to go in.  That would
allow trunk to be easily mergable *into* the branch for all except the
final autogenerated change.  We'd avoid introducing lots of "const_"
typedefs, the only weird thing would be (until the final patch goes in)
having "gimple" implicitly be a ptr, whereas the subclasses would
require explicit ptr syntax.

A merging strategy could be:
 - keep the branch hand-written, with ChangeLog.gimple-classes files
 - develop on branch, fixing more accessors, posting to gcc-patches
 - merge from trunk to the branch periodically
 - when ready, merge from branch to trunk in one commit
 - apply the final autogenerated change of "gimple" to "gimple *" in one
commit to trunk.

That way everything is diffable, and the awkward autogenerated megapatch
is isolated as one change.

Thoughts?

Thanks for reading this far!

Dave



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