Re: [RFC] "single step" atomic instruction sequences as a whole on PPC

Ulrich Weigand
2007-05-07T18:11:54Z

Luis Machado wrote:

> Thanks for the comments. I'm still getting used to the details of the
> coding standard.
>
> The patch has been updated. I hope i've addressed all the coding format
> issues this time.

It's much better, thanks. Unfortunately there's still a number of
issues -- thanks for you patience in dealing with those.

> The handling of multiple conditional branches was modified so that when
> GDB finds such a situation it just silently falls back to the standard
> way of doing single-step. That way we won't have too much text on the
> screen in case this situation happens often.

I guess that's OK with me. I'm wondering why we need the one message
that's still in there then -- I'd say either we warn whenever we find
a sequence we don't understand, or we never warn.

> + /* Assume all atomic sequences start with an lwarx or ldarx instruction. */
> + if ((insn & LWARX_MASK) != LWARX_INSTRUCTION
> + && (insn & LWARX_MASK) != LDARX_INSTRUCTION)
> + return 0;
"return" is now indented two spaces too much.

> + /* Assume that no atomic sequence is longer than "atomic_sequence_length
Missing closing "

> + /* Assume that there is at most one conditional branch in the atomic
> + sequence. If a conditional branch is found, put a breakpoint in
Two spaces after '.'

> + warning (_("Tried to step over an atomic sequence of instructions at %s\n \
> + but could not find the end of the sequence."),
If we keep the warning, we need to remove the extra spaces at the beginning of
the second line. See e.g. _initialize_printcmd for examples of long messages.

> +/* AIX does not support PT_STEP. Simulate it. */
Two spaces not just after the last '.', but after each of them.

> + /* Handles single stepping of atomic sequences */
Comment should end in '.'.

Otherwise, this looks technically OK to me. Once the coding style issues
and the warning question are resolved, I think it can be committed.

Bye,
Ulrich

--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com