Ping: [PATCH v2] x86-64: fix ZMM register state tracking

Jan Beulich
2018-10-29T10:31:54Z

>>> On 10.10.18 at 17:12, wrote:
> The three AVX512 state components are entirely independent - one being
> in its "init state" has no implication whatsoever on either of the other
> two. Fully separate X86_XSTATE_ZMM_H and X86_XSTATE_ZMM handling, to
> prevent upper halves of the upper 16 ZMM registers to display as if they
> were zero (when they aren't) after e.g. VZEROALL/VZEROUPPER.
>
> gdb/
> 2018-10-10 Jan Beulich <jbeulich@suse.com>
>
> * i387-tdep.c (i387_supply_xsave): Split handling of
> X86_XSTATE_ZMM_H and X86_XSTATE_ZMM.
> (i387_collect_xsave): Likewise.
>
> gdb/testsuite/
> 2018-10-10 Simon Marchi <simon.marchi@polymtl.ca>
>
> * testsuite/gdb.arch/i386-avx512.c,
> testsuite/gdb.arch/i386-avx512.exp: Add 7th test.
>
> ---
> v2: Attach comments to zmm_endlo_regnum declarations. Add testcase
> provided by Simon.
>
> --- a/gdb/i387-tdep.c
> +++ b/gdb/i387-tdep.c
> @@ -924,6 +924,12 @@ i387_supply_xsave (struct regcache *regc
> struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> const gdb_byte *regs = (const gdb_byte *) xsave;
> int i;
> + /* In 64-bit mode the split between "low" and "high" ZMM registers is at
> + ZMM16. Outside of 64-bit mode there are no "high" ZMM registers at
> all.
> + Precalculate the number to be used for the split point, with the all
> + registers in the "low" portion outside of 64-bit mode. */
> + unsigned int zmm_endlo_regnum = I387_ZMM0H_REGNUM (tdep)
> + + std::min (tdep->num_zmm_regs, 16);
> ULONGEST clear_bv;
> static const gdb_byte zero[I386_MAX_REGISTER_SIZE] = { 0 };
> enum
> @@ -1002,7 +1008,8 @@ i387_supply_xsave (struct regcache *regc
> return;
>
> case avx512_zmm_h:
> - if ((clear_bv & (X86_XSTATE_ZMM_H | X86_XSTATE_ZMM)))
> + if ((clear_bv & (regnum < zmm_endlo_regnum ? X86_XSTATE_ZMM_H
> + : X86_XSTATE_ZMM)))
> regcache->raw_supply (regnum, zero);
> else
> regcache->raw_supply (regnum,
> @@ -1080,21 +1087,17 @@ i387_supply_xsave (struct regcache *regc
> }
> }
>
> - /* Handle the upper ZMM registers. */
> - if ((tdep->xcr0 & (X86_XSTATE_ZMM_H | X86_XSTATE_ZMM)))
> + /* Handle the upper halves of the low 8/16 ZMM registers. */
> + if ((tdep->xcr0 & X86_XSTATE_ZMM_H))
> {
> - if ((clear_bv & (X86_XSTATE_ZMM_H | X86_XSTATE_ZMM)))
> + if ((clear_bv & X86_XSTATE_ZMM_H))
> {
> - for (i = I387_ZMM0H_REGNUM (tdep);
> - i < I387_ZMMENDH_REGNUM (tdep);
> - i++)
> + for (i = I387_ZMM0H_REGNUM (tdep); i < zmm_endlo_regnum; i++)
> regcache->raw_supply (i, zero);
> }
> else
> {
> - for (i = I387_ZMM0H_REGNUM (tdep);
> - i < I387_ZMMENDH_REGNUM (tdep);
> - i++)
> + for (i = I387_ZMM0H_REGNUM (tdep); i < zmm_endlo_regnum; i++)
> regcache->raw_supply (i,
> XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, i));
> }
> @@ -1119,11 +1122,13 @@ i387_supply_xsave (struct regcache *regc
> }
> }
>
> - /* Handle the YMM_AVX512 registers. */
> + /* Handle the upper 16 ZMM/YMM/XMM registers (if any). */
> if ((tdep->xcr0 & X86_XSTATE_ZMM))
> {
> if ((clear_bv & X86_XSTATE_ZMM))
> {
> + for (i = zmm_endlo_regnum; i < I387_ZMMENDH_REGNUM (tdep); i++)
> + regcache->raw_supply (i, zero);
> for (i = I387_YMM16H_REGNUM (tdep);
> i < I387_YMMH_AVX512_END_REGNUM (tdep);
> i++)
> @@ -1135,6 +1140,9 @@ i387_supply_xsave (struct regcache *regc
> }
> else
> {
> + for (i = zmm_endlo_regnum; i < I387_ZMMENDH_REGNUM (tdep); i++)
> + regcache->raw_supply (i,
> + XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, i));
> for (i = I387_YMM16H_REGNUM (tdep);
> i < I387_YMMH_AVX512_END_REGNUM (tdep);
> i++)
> @@ -1341,6 +1349,9 @@ i387_collect_xsave (const struct regcach
> gdb_byte raw[I386_MAX_REGISTER_SIZE];
> ULONGEST initial_xstate_bv, clear_bv, xstate_bv = 0;
> unsigned int i;
> + /* See the comment in i387_supply_xsave(). */
> + unsigned int zmm_endlo_regnum = I387_ZMM0H_REGNUM (tdep)
> + + std::min (tdep->num_zmm_regs, 16);
> enum
> {
> x87_ctrl_or_mxcsr = 0x1,
> @@ -1441,9 +1452,8 @@ i387_collect_xsave (const struct regcach
> i < I387_MPXEND_REGNUM (tdep); i++)
> memset (XSAVE_MPX_ADDR (tdep, regs, i), 0, 8);
>
> - if ((clear_bv & (X86_XSTATE_ZMM_H | X86_XSTATE_ZMM)))
> - for (i = I387_ZMM0H_REGNUM (tdep);
> - i < I387_ZMMENDH_REGNUM (tdep); i++)
> + if ((clear_bv & X86_XSTATE_ZMM_H))
> + for (i = I387_ZMM0H_REGNUM (tdep); i < zmm_endlo_regnum; i++)
> memset (XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, i), 0, 32);
>
> if ((clear_bv & X86_XSTATE_K))
> @@ -1453,6 +1463,8 @@ i387_collect_xsave (const struct regcach
>
> if ((clear_bv & X86_XSTATE_ZMM))
> {
> + for (i = zmm_endlo_regnum; i < I387_ZMMENDH_REGNUM (tdep); i++)
> + memset (XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, i), 0, 32);
> for (i = I387_YMM16H_REGNUM (tdep);
> i < I387_YMMH_AVX512_END_REGNUM (tdep); i++)
> memset (XSAVE_YMM_AVX512_ADDR (tdep, regs, i), 0, 16);
> --- a/gdb/testsuite/gdb.arch/i386-avx512.c
> +++ b/gdb/testsuite/gdb.arch/i386-avx512.c
> @@ -249,6 +249,13 @@ main (int argc, char **argv)
> move back to array and check values. */
> move_zmm_data_to_memory ();
> asm ("nop"); /* sixth breakpoint here */
> +
> + asm ("vpternlogd $0xff, %zmm0, %zmm0, %zmm0");
> +#ifdef __x86_64__
> + asm ("vpternlogd $0xff, %zmm0, %zmm0, %zmm16");
> +#endif
> + asm ("vzeroupper");
> + asm ("nop"); /* seventh breakpoint here */
> }
>
> return 0;
> --- a/gdb/testsuite/gdb.arch/i386-avx512.exp
> +++ b/gdb/testsuite/gdb.arch/i386-avx512.exp
> @@ -174,3 +174,13 @@ for { set r 0 } { $r < $nr_regs } { incr
> ".. = \\{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"
> }
> +
> +gdb_test "break [gdb_get_line_number "seventh breakpoint here"]" \
> + "Breakpoint .* at .*i386-avx512.c.*" \
> + "set seventh breakpoint in main"
> +gdb_continue_to_breakpoint "continue to seventh breakpoint in main"
> +gdb_test "print \$zmm0.v16_int32" "= {-1, -1, -1, -1, 0 <repeats 12
> times>}"
> +
> +if { $nr_regs >= 16 } {
> + gdb_test "print \$zmm16.v16_int32" "= {-1 <repeats 16 times>}"
> +}
>
>
>
>