Re: PATCH: libgcc2/frame.c

[Mark Mitchell <mark at codesourcery dot com>]

>>>>> "Nathan" == Nathan Sidwell <nathan@acm.org> writes:

    Nathan> Hi, here's a patch to frame.c which fixes the bug report
    Nathan> at
    Nathan> http://egcs.cygnus.com/ml/gcc-bugs/1999-08/msg00918.html. fde_split
    Nathan> makes use of the gcc variable sized array extension to
    Nathan> hold a chain of linear FDE's. This can make the stack
    Nathan> frame rather large, which is bad, and doubly so for
    Nathan> multithreaded applications. The patch makes use of the
    Nathan> erratic array to hold this chain, during its generation,
    Nathan> and then fills the erratic array with unordered FDE's in a
    Nathan> final pass. 

Good idea.  This is an important quality-of-implementation
improvement.

    Nathan> it is filled with the latter. The algorithm is type-based
    Nathan> alias safe because at the split pass, we look at an entry
    Nathan> as a `struct fde **', to determine whether we have an
    Nathan> erratic or linear entry. For the former, we write a
    Nathan> `struct fde *' into the erratic array, which, in the worst
    Nathan> case (when the original LINEAR array begins with an
    Nathan> erratic sequence), will be the entry we just peeked
    Nathan> at. These two accesses can't be reordered, as that would
    Nathan> violate causality.

Why not just make fde_vector look like:

  struct fde_vector {
    union {
      fde **array;
      fde ***chain;
    } u;
    size_t count;
  };

Wouldn't that make it a little clearer what's going on?  

In fact, I'd just as soon use `malloc' to allocate a fresh array.  Is
this function on the critical path in some way?  I don't think so;
are frames sorted just once.  We're already malloc'ing two arrays
(linear, erratic); why not just malloc a third and be done with it?

I like simple programs.

    Nathan> ok?

Yes, basically.  I'll let you decide about using the union
vs. allocating a new array.  Please post the final patch after your
check-in.  And I'll look at the follow-on patches at that point.

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