Re: [PATCH 0/2] Support the new PPC476 processor

Sérgio Durigan Júnior
2009-12-20T15:09:53Z

Hi Joel,

On Sun, Dec 20, 2009 at 11:15 AM, Joel Brobecker <brobecker@adacore.com> wrote:

> I did a quick pass over the patch that you sent, and I pretty much
> agree with the comments made by Eli.  My general comment, is that
> the patch is too big for me to really digest as is (3,000 lines in
> total!)- maybe it's just me being preoccupied with some many things
> going on at the same time.  If you'd like to help me review your changes,
> I suggest we take a different approach than the one you took to present
> these patches: I'd like to have a set of independent patches that implement
> each feature independently (that way, I don't have to try to determine
> which feature each hunk applies to). We don't have to have them all,
> in fact, I'd feel less overwhelmed if we started with just one feature.
> For instance, we could look at hardware-accelerated watchpoint conds.
> We could look at watchpoint ranges, but we are still discussing the user
> interface...

I am sorry about that. When I sent the patch, I decided to use the
same approach that I used for the catch syscall series, but apparently
it wasn't a good idea.

I will see what I can do in order to get the patches divided the way
you want; I don't think it will be hard to accomplish that. Thiago is
probably going to resubmit the patches, since I'm not working at IBM
anymore (that's the reason why I haven't replied to Eli's messages, by
the way).

> One stylistic comment is that I'm having a hard time with the use of
> "point" to mean either breakpoint or watchpoint. I wonder if we could
> find something else, but nothing really comes to mind. I am tempted
> to say that watchpoints are really data breakpoints while breakpoints
> are instruction breakpoints (this is how some documents that I read
> called watchpoints: data breakpoints), but perhaps using breakpoint
> in this case is going to just be too confusing in the GDB context.
> Hmmmm....

When I decided to use this term, I based that decision on the fact
that gdbserver uses the same nomenclature: for example, it has a
variable called `debug_hw_points', the methods `insert_point' and
`remove_point', etc. But of course, if you think this term is
confusing, we should be able to find a better one :-).

Thank you for the pre-review!

Sérgio.