x86-64: fix ZMM register state tracking

Message ID 0e0d9f23-2cbf-eb5a-64fa-6cda3392053a@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Sept. 25, 2018, 3:28 a.m. UTC
  On 2018-09-18 09:37 AM, Jan Beulich wrote:
>>>> On 10.09.18 at 15:01, <simon.marchi@polymtl.ca> wrote:
>> On 2018-09-10 07:25, Jan Beulich wrote:
>>>>>> Simon Marchi <simon.marchi@ericsson.com> 09/08/18 1:13 AM >>>
>>>> Would it be possible to update or create a test to exercise that?
>>>> arch-specific tests are in testsuite/gdb.arch.
>>>
>>> I'm sure it would be possible, but while I was happy to invest the time 
>>> to
>>> fix the actual bug (because it affects work I'm doing), I'm afraid I 
>>> don't have
>>> the time to learn how gdb test cases are to be constructed (I'm 
>>> familiar
>>> only with the binutils / gas side of things).
>>
>> I understand.  If you can provide:
>>
>>   - a minimal source file (C + assembly in this case, I suppose)
>>   - GDB commands to reproduce the bug
>>   - actual and expected output
> 
> Attached. vzero.s is the source file used (no C file needed). gdb.log
> is a transcript of a session with a broken gdb (the one installed on
> the system), while gdb.txt is a transcript for the fixed one that I've
> built myself.
> 
>> I can take care of turning it in a GDB test case.
> 
> Thanks.
> 
> Jan
> 
> 

Hi Jan,

Thanks for the instructions.  There is already a test covering AVX512
instructions, so I figured I would add it there.  However, I don't
have a processor that supports AVX512, so I'm unable to run the test.

Here's a patch, can you try to confirm that the test fails without the
fix and passes with the fix?  I probably screwed up somewhere, but it
should be pretty close.

You can run the test with

  $ make check TESTS="gdb.arch/i386-avx512.exp"

in the gdb/ build directory.  The gdb/testsuite/gdb.log file is useful
to look at in case something fails.

Feel free to integrate this in your eventual v2 if there is one, and
feel free to add a comment in the test to say what we are testing here,
as I am not too sure how to describe it.

Thanks!

Simon


From a167d21f452f6f3f19eac6623fa872fde1162128 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Mon, 24 Sep 2018 23:21:19 -0400
Subject: [PATCH] AVX512 test

---
 gdb/testsuite/gdb.arch/i386-avx512.c   | 5 +++++
 gdb/testsuite/gdb.arch/i386-avx512.exp | 8 ++++++++
 2 files changed, 13 insertions(+)
  

Comments

Jan Beulich Sept. 25, 2018, 3:04 p.m. UTC | #1
>>> On 25.09.18 at 05:28, <simon.marchi@ericsson.com> wrote:
> On 2018-09-18 09:37 AM, Jan Beulich wrote:
> Thanks for the instructions.  There is already a test covering AVX512
> instructions, so I figured I would add it there.  However, I don't
> have a processor that supports AVX512, so I'm unable to run the test.
> 
> Here's a patch, can you try to confirm that the test fails without the
> fix and passes with the fix?  I probably screwed up somewhere, but it
> should be pretty close.

There are two issues here: First of all, unrelated to this patch, the
construct around line 95 in i386-avx512.exp should look like

if [is_amd64_regs_target] {
    set nr_regs 32
} else {
    set nr_regs 8
}

Of course this also affects other tests in here, but without this correction
the loop you add does nothing at all.

And then that very loop and the i386-avx512.c addition are not in sync,
and I'm not sure which way you meant it to be: Either in the C file all 16
upper ZMM registers need to be set identically (not just ZMM16), or
there should be no loop.

Furthermore I think the C code addition and hence the test will need to
be x86-64-specific, as registers ZMM8 and higher are inaccessible in
32-bit mode.

So what I can confirm at this point is that with the fix in place there's
one less new failure from the test than with the fix no in place.

Jan
  

Patch

diff --git a/gdb/testsuite/gdb.arch/i386-avx512.c b/gdb/testsuite/gdb.arch/i386-avx512.c
index 9349f09d62e..537adb3ab86 100644
--- a/gdb/testsuite/gdb.arch/i386-avx512.c
+++ b/gdb/testsuite/gdb.arch/i386-avx512.c
@@ -249,6 +249,11 @@  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\n"
+	   "vpternlogd $0xff, %zmm0, %zmm0, %zmm16\n"
+	   "vzeroupper\n");
+      asm ("nop"); /* seventh breakpoint here  */
     }

   return 0;
diff --git a/gdb/testsuite/gdb.arch/i386-avx512.exp b/gdb/testsuite/gdb.arch/i386-avx512.exp
index d806d5f9a94..de2f62c3e1f 100644
--- a/gdb/testsuite/gdb.arch/i386-avx512.exp
+++ b/gdb/testsuite/gdb.arch/i386-avx512.exp
@@ -174,3 +174,11 @@  for { set r 0 } { $r < $nr_regs } { incr 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"
 }
+
+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"
+for { set r 16 } { $r < $nr_regs } { incr r } {
+    gdb_test "print \$zmm${r}.v16_int32" "{-1 <repeats 16 times>}"
+}