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]

Re: C++: Fixing vtable thunks



Martin --

  Wow, that's a serious patch!  I think I'm a little new at the job of
maintainer to approve something quite that significant.

  I haven't had a chance to look at this in a lot of detail, and I may
not for a bit.  But, here are my initial thoughts.

  o Your .gz file causes my version of gunzip to segfault.  So, I 
    can't really look at the patch. Bummer.

  o I'd like to see the binary compatibility worked out before we put
    this in.  Otherwise, we may just introduce another incompatibility
    whereby people using the new thunks then can't link with stuff
    created by the binary-compatible version of the new thunks.

  o Stylistically, I'd prepare -fvtable-thunks=2, rather than
    -fv1-thunks, etc.  Let's just -fvtable-thunks default to the 
    current behavior, and allow the user to override it there, rather
    than adding a new option.

  o I'd like you to document both the old and new vtable and thunk ABI
    and how it works.  That's something I've never been 100% clear on,
    and I think that you're probably now the planet's leading expert
    on how g++ deals with these issues.  I can't tell if you did this
    or not, since I couldn't really look at the patch.  But, I think
    this should be done in complete detail: what data structures are
    used, what algorithm is used to call a virtual function using
    them, what aspects are machine dependent or not, what a machine
    description needs to do to implement thunks, etc.  I know this is
    a chore, but it will serve several purposes:
    
    - We'll understand better what you're doing, and that will
      make it possible to read your patch more easily.

    - We'll all be more likely to find any design bugs before
      we deploy this stuff in the wild.  I'm not implying that you
      didn't do everything perfectly, but it's bad to have to change
      the ABI, so we want to be extra special careful, even if that
      seems a little disrespectful.

    - If and when something goes wrong, or we're considering a change
      we'll be clear on what we can do without breaking the ABI, or
      why the code we're generating isn't right.

    - GDB, Pure, and other folks that try to deal with object files
      generated by GCC could more easily figure out what's going on.

    Some of this information may already be littered throughout the
    code and gxxint.texi, and maybe even the main gcc info pages, but
    it's certainly not all together in an easy to find place.

  BTW, although I don't request that *you* do this necessarily, why on
earth isn't gxxint.texi part of the standard gcc manual?  These
details are analagous to the sections on RTL in the manual, and it
would be nice if these were installed automatically.  Would one of our
configuration experts (Kaveh?) like to make this work?  (We should do
this only if LANGUAGES includes c++).  I think this would be a
considerable service.

-- 
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com


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