Re: [PATCHv2] aarch64: detect atomic sequences like other ll/sc architectures

[Kyle McMartin <kmcmarti@redhat.com>]

On Thu, Mar 27, 2014 at 06:07:14AM -0700, Joel Brobecker wrote:
> A few comments on the testcase. I have no further comments on the code
> itself.
> 
> > +#include <stdio.h>
> 
> Do you really need stdio.h, here? You do not seem to be making
> any function call in your function, so I do not see why it would
> be needed. On the other hand, having a dependency on stdio.h means
> that the testcase will not compile on many targets (eg: bare metal).
> 

Not that I'm aware of, I'd re-added it as I added some printfs for
debugging while writing the testcase and forgot to remove it again. (The
PPC64 one has it for some reason as well, likely the same reason.)

> > +
> > +int main()
> 
> Can you use "(void)" instead of "()"?
> 

Absolutely.

> > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $compile_flags] != "" } {
> > +    unsupported "Testcase compile failed."
> > +    return -1
> > +}
> > +gdb_exit
> > +gdb_start
> > +gdb_reinitialize_dir $srcdir/$subdir
> > +gdb_load ${binfile}
> 
> Can you use standard_testfile and prepare_for_testing? See our testcase
> cookbook page at:
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook
> 
> It should replace all of the above after the "if istarget"...
> 

OK, testing this out now.

> > +if ![runto_main] then {
> > +    perror "Couldn't run to breakpoint"
> > +    continue
> 
> Also according to the cookbook, use:
> 
>     untested "could not run to main"
>     return -1
> 
> IIRC, with tcl, there isn't a huge distinction in this case between
> continue and return -1, but might as well follow the cookbook.
> 
> > +gdb_test continue "Continuing.*Breakpoint $decimal.*" \
> > +  "Continue until breakpoint"
> > +
> > +gdb_test next ".*__asm __volatile.*" \
> > +  "Step through the ldxr/stxr sequence"
> > +
> > +gdb_test next ".*return dword.*" \
> > +  "Stepped through sequence through conditional branch"
> 
> Can you put the "continue"/"next" inside double-quotes.
> It looks like it's all the same to tcl, but I think it'll make
> things more consistent and also allow editors to (syntax)-highlight
> those as strings...
> 

Will do.

regards, Kyle

> Thanks,
> -- 
> Joel