Re: [RFC/WIP PATCH 00/14] I/T sets (resend)

[Pedro Alves <palves@redhat.com>]

On 02/09/2012 08:18 AM, Tomas Östlund wrote:
> Resending since Pedro changed e-mail
> 
> --------
> 
> Hi!
> 
> I hope it is not too late to give feedback and perhaps influence this
> proposed feature. Let me first say that I think this is a very good
> initiative and I see a lot of potential for it.

It's not.  :-)

> This was only some examples to give you a quick overview from where we
> are coming.

Thanks!  This is largely in line with the work's original requirements.

> 
> Now some actual feedback:
> 
> On 01/-10/37 20:59, Pedro Alves wrote:
> [...]
>> Along the idea that we
>> need an intersection operator somehow, an idea I've been kicking in
>> the background, is to make all kinds of objects have the same stand,
>> and require, say, a one letter prefix to identify what kind of object
>> we're specifying.  E.g, i for inferior, p for process, t for thread, c
>> for core and a for Ada task.  In this scheme, the '.'  is really a set
>> intersection operator, and the ',' is the union operator.  I'm very
>> much inclined to try this route, but I'm also very interested in
>> learning other's opinions.  Here are examples:
>>
>> | Syntax                  | Meaning                                           |
>> |-------------------------+---------------------------------------------------|
>> | i1.t1                   | inferior 1, thread 1                              |
>> |-------------------------+---------------------------------------------------|
>> | p123                    | all (threads cores, etc.) of process with pid 123 |
>> |-------------------------+---------------------------------------------------|
>> | i1.t[1-100].c1-3        | threads 1 to 100 of iinferior 1, running on       |
>> |                         | cores 1 to 3                                      |
>> |-------------------------+---------------------------------------------------|
>> | i1.t*.c1-3,workers      | same as above, union with all in the user         |
>> |                         | defined set "workers"                             |
>> |-------------------------+---------------------------------------------------|
>> | workers.~c1-3           | All in the user defined set "workers", except     |
>> |                         | those on cores 1 to 3                             |
>> |-------------------------+---------------------------------------------------|
>> | i1.a1-3                 | Ada tasks 1 to 3 of inferior 1                    |
>> |-------------------------+---------------------------------------------------|
>> | workers.i1,crunchers.i2 | All workers of inferior 1, and                    |
>> |                         | all crunchers of inferior 2                       |
>> |-------------------------+---------------------------------------------------|
>>
>> WDYT?
> 
> I think this is a very good suggestion and a must in order to make all
> objects have the same stand as you say.

Great.  I actually implemented most of this.

See the v2 (latest version) of the series at:

 http://sourceware.org/ml/gdb-patches/2011-12/msg00550.html

>> The current implementation, borrowing heavilly from the HPD and the
>> syntax we first thought out (see Stan's link above), currently
>> implements the INF.TID form, and as a separate patch, the @core
>> extension.
> 
> After some googling I finally found what HPD refers to. I had never
> heard of it before and I'm curious what happened to the initiative?
> Anyway, I've read the specification and think it has a lot of good
> concepts, although it misses (intentionally?) to consider processor
> cores as first class elements. One thing I dislike though is the syntax
> for specifying sets using brackets ([]). Using the command line should
> not be harder that it has to be and using brackets is very awkward,
> especially on many keyboards with non-US layout. For example, on a
> Swedish keyboard, you have to use a combination of ALT GR + 8 to
> produce "[". Also I don't see the point of putting the set
> specification before the actual command. 

A agree.  On a Portuguese keyboard it's also painful.  And that's why
I got rid of []'s in v2.

For example compare the
> operation of doing "step dsp0, dsp1 and dsp4" with our debugger
> 
>> step dsp0 dsp1 dsp4
> 
> and using the HPD syntax:
> 
>> [dsp0,dsp1,dsp4] step
> 
> Which one is more natural and easier to type?

See v2.  :-)

> 
> Quite often in debugging, you reuse earlier debugger commands from
> command history and modify them to apply to some other set of cores or
> threads. This is also more easily done, if what commands should apply
> to, are placed at the end of a command.
> 
> We think syntax-wise can spaces or commas be used as separators between
> union members. It is easier to type, will cause fewer typing mistakes
> and it is also easier to visually distinguish between ' ' and '.' than
> between ',' and '.'.

The problem is one of ambiguity when you want to be able to specify
sets as an option, and support other options.  E.g.,

break -stop foo bar qux

-stop takes a set as argument.  Is the set "foo", or "foo bar"?  Perhaps
even "foo bar qux"?  You'd need to quote the set somehow, like

break -stop 'foo bar' qux.

which we were trying to avoid (just like []'s)

We could allow optional quoting.  Another alternative would be to use some
other characters.  I don't know, '.', and ',' seemed quite intuitive to me.  Maybe
you could try v2 out a bit.  It's still easily accessible in my git, as pointed
out in the v2 series intro.

> If there are historical reasons for supporting the 'postfix' HPD syntax
> we would like to extend the command syntax to support 'infix' as well
> as we have given examples of above.
> 
> The HPD syntax does not specify a rationale for putting the set
> specification before the command. What is the reason for that?

It kind of follows from the idea that you can also switch the
set focus permanently.  With "itfocus" in v1, or "scope" in v2.

You define your commands to apply to the current
scope.  So:

(gdb) itfocus foo / scope foo
(gdb) step

is the same as

(gdb) [foo] step  (v1)

or

(gdb) scope foo step  (v2)

IOW, you can think of prefix scope as a shorthand.  BTW, the v2 syntax is
quite like TotalView's (they call theirs "dfocus", IIRC).

> 
>>
>> The following examples exhibit some possible uses of sets:
>>
>> |-------------------------------------------+----------------------------------|
>> | Commands                                  | Behaviour                        |
>> |-------------------------------------------+----------------------------------|
>> | (gdb) step [.34-59]                       | Single-step the current          |
>> |<lines stepped over>                       | process's threads whose          |
>> | (gdb)                                     | numbers are in the range 34      |
>> |                                           | through 59, inclusive            |
> 
> Here you are not using the HPD syntax with putting the set before the
> command. Why? Although I like this one better. Now let's just skip the
> brackets :-).

:-)  That's what you get while your mind isn't completely sorted out yet.
Please do see the v2 series.


> 
> 
>> |-------------------------------------------+----------------------------------|
>> | (gdb) step [@2]                           | Single-step all threads on       |
>> |<line stepped over>                        | core 2                           |
>> | (gdb)                                     |                                  |
> 
> Hmm, if this means step *all* threads on core 2, how would you express
> the scenario in our debugger, where you can step a single machine code
> instruction on a specified core?
> With your proposed syntax using a letter prefix, should it not instead
> be
> 
> (gdb) step [t*.c2]

Yes.  The proposed syntax hadn't been implemented yet on the v1.  I had written
the new syntax as a proposal of something to be changed after v1.  And I did it for v2.

> 
>>     However, what if the user does:
>>
>>     [foo]>  itset [1.2]
>>     [1.2]>  step
>>
>>     Here the user has explicitly said it only wants to focus on thread 2.
>>     What should the step above resume freely?  HPD seems to suggest that
>>     the global setting should also be respected, which by default means
>>     the whole process is resumed.
>>
>>     Let's get back to focusing on the whole process:
>>
>>     [1.2]>  itset [1.*]
>>     focus set to [1.*]
>>     [1.*]>  step
>>
>>     And imagine even that we're only debugging inferior 1.
>>     Now, according to HPD, this would step each and every thread of the
>>     process.  It's equivalent to issuing:
>>
>>     [some_default_focus] [1.*]           step
>>       ^^^^^^^^^^^^^^^^    ^^^
>>         prompt            explicit set   cmd
>>
>>
>>     But, this conflicts with GDB's concept of a selected thread, and
>>     will be counter to all of our user's expectations...  Worse, it's
>>     unimplementable on targets that don't do non-stop, so we'd end up
>>     with yet another incompatible mode.  I'd very much like to merge
>>     all modes, not add another!
> 
> We totally agree! It is much better to define a flexible command syntax
> that directly supports executing in the execution modes without having
> to do any mode switching.
> 
> From our experience we have not seen the need for specifying multiple
> 'execution masters' nor are the semantics of such execution operations
> clear. So if we for example express that simultaneously step 3 cores
> and let all other cores run for that duration, does execution stop when
> the first 'master core' stops ?. Or do we run until all master cores
> stop (but that would mean they are no longer in sync i.e. have run the
> same amount of cycles)? What happens if some master core
> never stops? But by specifying a single execution master, the behavior
> is much more predicable and synchronous.

That's interesting.  Thanks!

-- 
Pedro Alves