Re: [PATCH V5 1/3] Add AVX512 registers support to GDB.

Pedro Alves
2014-04-24T11:33:55Z

On 04/23/2014 02:02 PM, Michael Sturm wrote:
> +
> +for { set r 0 } { $r < $nr_regs } { incr r } {
> + set val [expr $r + 1]
> + gdb_test "print/x k_data\[$r\]" \
> + ".. = 0x$val$val$val$val" \
> + "check contents of k_data\[$r\]"
> +
> +gdb_test "break [gdb_get_line_number "third breakpoint here"]" \
> + "Breakpoint .* at .*i386-avx512.c.*" \
> + "set third breakpoint in main"
> +gdb_continue_to_breakpoint "continue to third breakpoint in main"
> +

This bit wasn't addressed -- indentation here and all the way through
below still isn't right.

Ah, actually, it's not really the indentation that is missing.
It's a '}' to close the for loop above that is missing. So it
looks like the "check contents of k_data\[$r\]" test is only
running for $r == 0. Then the remainder of the tests
don't actually run $nr_regs times, because there are other loops
within that broken loop that also use $r as iterator:

> +for { set r 0 } { $r < $nr_regs } { incr r } {

> + gdb_test "print zmm_data\[$r\]" \
> + ".. = \\{f = \\{[expr $r + 30], [expr $r.125 + 30], [expr $r.25 + 20], [expr $r.375 + 20], [expr $r.5 + 10], [expr $r.625 + 10], [expr $r.75 + 10], [expr $r.875 + 10]\\}\\}.*" \
> + "check contents of zmm_data\[$r\] after writing XMM regs"
> +}
> +
> +}
> +

This last "}" is closing the broken for loop. Seems to me it's misplaced.

Other than that, looks fine to me. Please push with those issues fixed.

--
Pedro Alves