* [PATCH] [gdb/tdep] Fix avx512 -m32 support in gdbserver
@ 2021-11-30 9:11 Tom de Vries
2021-12-02 16:43 ` Keith Seitz
0 siblings, 1 reply; 5+ messages in thread
From: Tom de Vries @ 2021-11-30 9:11 UTC (permalink / raw)
To: gdb-patches
PR27257 reports a problem that can be reproduced as follows:
- use x86_64 machine with avx512 support
- compile a hello world with -m32 to a.out
- start a gdbserver session with a.out
- use gdb to connect to the gdbserver session
This makes us run into:
...
Listening on port 2346
Remote debugging from host ::1, port 34940
src/gdbserver/regcache.cc:257: \
A problem internal to GDBserver has been detected.
Unknown register zmm16h requested
...
The problem is that i387_xsave_to_cache in gdbserver/i387-fp.cc can't find a
register zmm16h in the register cache.
To understand how this happens, first some background.
SSE has 16 128-bit wide xmm registers.
AVX extends the SSE registers set as follows:
- it extends the 16 existing 128-bit wide xmm registers to 256-bit wide ymm
registers.
AVX512 extends the AVX register set as follows:
- it extends the 16 existing 256-bit wide ymm registers to 512-bit wide zmm
registers.
- it adds 16 additional 512-bit wide zmm registers (with corresponding ymm and
xmm subregisters added as well)
However, in 32-bit mode, there are only 8 xmm/ymm/zmm registers.
The problem we're running into is that gdbserver/i387-fp.cc uses these
constants to describe the size of the register file:
...
static const int num_avx512_zmmh_low_registers = 16;
static const int num_avx512_zmmh_high_registers = 16;
static const int num_avx512_ymmh_registers = 16;
static const int num_avx512_xmm_registers = 16;
...
which are all incorrect for the 32-bit case.
Fix this by replacing the constants with variables that have the appropriate
values in 64-bit and 32-bit mode.
Tested on x86_64-linux with native and unix/-m32.
---
gdbserver/i387-fp.cc | 50 ++++++++++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 13 deletions(-)
diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
index 6d9b62ed8ce..9b07ded592d 100644
--- a/gdbserver/i387-fp.cc
+++ b/gdbserver/i387-fp.cc
@@ -23,10 +23,6 @@
static const int num_mpx_bnd_registers = 4;
static const int num_mpx_cfg_registers = 2;
static const int num_avx512_k_registers = 8;
-static const int num_avx512_zmmh_low_registers = 16;
-static const int num_avx512_zmmh_high_registers = 16;
-static const int num_avx512_ymmh_registers = 16;
-static const int num_avx512_xmm_registers = 16;
static const int num_pkeys_registers = 1;
/* Note: These functions preserve the reserved bits in control registers.
@@ -256,14 +252,22 @@ void
i387_cache_to_xsave (struct regcache *regcache, void *buf)
{
struct i387_xsave *fp = (struct i387_xsave *) buf;
+ bool amd64 = register_size (regcache->tdesc, 0) == 8;
int i;
unsigned long val, val2;
unsigned long long xstate_bv = 0;
unsigned long long clear_bv = 0;
char raw[64];
char *p;
+
/* Amd64 has 16 xmm regs; I386 has 8 xmm regs. */
- int num_xmm_registers = register_size (regcache->tdesc, 0) == 8 ? 16 : 8;
+ int num_xmm_registers = amd64 ? 16 : 8;
+ /* AVX512 extends the existing xmm/ymm registers to a wider mode: zmm. */
+ int num_avx512_zmmh_low_registers = num_xmm_registers;
+ /* AVX512 adds 16 extra regs in Amd64 mode, but none in I386 mode.*/
+ int num_avx512_zmmh_high_registers = amd64 ? 16 : 0;
+ int num_avx512_ymmh_registers = amd64 ? 16 : 0;
+ int num_avx512_xmm_registers = amd64 ? 16 : 0;
/* The supported bits in `xstat_bv' are 8 bytes. Clear part in
vector registers if its bit in xstat_bv is zero. */
@@ -452,7 +456,9 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
/* Check if any of ZMM16H-ZMM31H registers are changed. */
if ((x86_xcr0 & X86_XSTATE_ZMM))
{
- int zmm16h_regnum = find_regno (regcache->tdesc, "zmm16h");
+ int zmm16h_regnum = (num_avx512_zmmh_high_registers == 0
+ ? -1
+ : find_regno (regcache->tdesc, "zmm16h"));
for (i = 0; i < num_avx512_zmmh_high_registers; i++)
{
@@ -469,7 +475,9 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
/* Check if any XMM_AVX512 registers are changed. */
if ((x86_xcr0 & X86_XSTATE_ZMM))
{
- int xmm_avx512_regnum = find_regno (regcache->tdesc, "xmm16");
+ int xmm_avx512_regnum = (num_avx512_xmm_registers == 0
+ ? -1
+ : find_regno (regcache->tdesc, "xmm16"));
for (i = 0; i < num_avx512_xmm_registers; i++)
{
@@ -486,7 +494,9 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
/* Check if any YMMH_AVX512 registers are changed. */
if ((x86_xcr0 & X86_XSTATE_ZMM))
{
- int ymmh_avx512_regnum = find_regno (regcache->tdesc, "ymm16h");
+ int ymmh_avx512_regnum = (num_avx512_ymmh_registers == 0
+ ? -1
+ : find_regno (regcache->tdesc, "ymm16h"));
for (i = 0; i < num_avx512_ymmh_registers; i++)
{
@@ -710,12 +720,20 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
{
struct i387_xsave *fp = (struct i387_xsave *) buf;
struct i387_fxsave *fxp = (struct i387_fxsave *) buf;
+ bool amd64 = register_size (regcache->tdesc, 0) == 8;
int i, top;
unsigned long val;
unsigned long long clear_bv;
gdb_byte *p;
- /* Amd64 has 16 xmm regs; I386 has 8 xmm regs. */
- int num_xmm_registers = register_size (regcache->tdesc, 0) == 8 ? 16 : 8;
+
+ /* Amd64 has 16 xmm regs; I386 has 8 xmm regs. */
+ int num_xmm_registers = amd64 ? 16 : 8;
+ /* AVX512 extends the existing xmm/ymm registers to a wider mode: zmm. */
+ int num_avx512_zmmh_low_registers = num_xmm_registers;
+ /* AVX512 adds 16 extra regs in Amd64 mode, but none in I386 mode.*/
+ int num_avx512_zmmh_high_registers = amd64 ? 16 : 0;
+ int num_avx512_ymmh_registers = amd64 ? 16 : 0;
+ int num_avx512_xmm_registers = amd64 ? 16 : 0;
/* The supported bits in `xstat_bv' are 8 bytes. Clear part in
vector registers if its bit in xstat_bv is zero. */
@@ -845,9 +863,15 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
if ((x86_xcr0 & X86_XSTATE_ZMM) != 0)
{
- int zmm16h_regnum = find_regno (regcache->tdesc, "zmm16h");
- int ymm16h_regnum = find_regno (regcache->tdesc, "ymm16h");
- int xmm16_regnum = find_regno (regcache->tdesc, "xmm16");
+ int zmm16h_regnum = (num_avx512_zmmh_high_registers == 0
+ ? -1
+ : find_regno (regcache->tdesc, "zmm16h"));
+ int ymm16h_regnum = (num_avx512_ymmh_registers == 0
+ ? -1
+ : find_regno (regcache->tdesc, "ymm16h"));
+ int xmm16_regnum = (num_avx512_xmm_registers == 0
+ ? -1
+ : find_regno (regcache->tdesc, "xmm16"));
if ((clear_bv & X86_XSTATE_ZMM) != 0)
{
base-commit: ae8e528122be3e0344b2ee533b4b050389e8dcad
--
2.31.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [gdb/tdep] Fix avx512 -m32 support in gdbserver
2021-11-30 9:11 [PATCH] [gdb/tdep] Fix avx512 -m32 support in gdbserver Tom de Vries
@ 2021-12-02 16:43 ` Keith Seitz
2021-12-02 17:00 ` Tom de Vries
0 siblings, 1 reply; 5+ messages in thread
From: Keith Seitz @ 2021-12-02 16:43 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 11/30/21 01:11, Tom de Vries via Gdb-patches wrote:
> PR27257 reports a problem that can be reproduced as follows:
> - use x86_64 machine with avx512 support
> - compile a hello world with -m32 to a.out
> - start a gdbserver session with a.out
> - use gdb to connect to the gdbserver session
>
> This makes us run into:
> ...
> Listening on port 2346
> Remote debugging from host ::1, port 34940
> src/gdbserver/regcache.cc:257: \
> A problem internal to GDBserver has been detected.
> Unknown register zmm16h requested
> ...
I was recently investigating this issue, too. You and I have
converged on the same conclusion and solution.
> Tested on x86_64-linux with native and unix/-m32.
I am going to assume you've tested with native-{,extended-}gdbserver
/-m32 on AVX512 (and without)?
Since I happened to have access to a machine where this is
a problem, I've also tested the patch, and the results there
are good.
This patch is really a giant step in the right direction.
[-m32 gdbserver tests on AVX512 were rather useless before this]
Thanks,
Keith
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [gdb/tdep] Fix avx512 -m32 support in gdbserver
2021-12-02 16:43 ` Keith Seitz
@ 2021-12-02 17:00 ` Tom de Vries
2021-12-02 17:02 ` Keith Seitz
0 siblings, 1 reply; 5+ messages in thread
From: Tom de Vries @ 2021-12-02 17:00 UTC (permalink / raw)
To: Keith Seitz, gdb-patches
On 12/2/21 5:43 PM, Keith Seitz wrote:
> On 11/30/21 01:11, Tom de Vries via Gdb-patches wrote:
>> PR27257 reports a problem that can be reproduced as follows:
>> - use x86_64 machine with avx512 support
>> - compile a hello world with -m32 to a.out
>> - start a gdbserver session with a.out
>> - use gdb to connect to the gdbserver session
>>
>> This makes us run into:
>> ...
>> Listening on port 2346
>> Remote debugging from host ::1, port 34940
>> src/gdbserver/regcache.cc:257: \
>> A problem internal to GDBserver has been detected.
>> Unknown register zmm16h requested
>> ...
>
> I was recently investigating this issue, too. You and I have
> converged on the same conclusion and solution.
>
Ah, I see.
>> Tested on x86_64-linux with native and unix/-m32.
> I am going to assume you've tested with native-{,extended-}gdbserver
> /-m32 on AVX512 (and without)?
>
Well, I've tested with native and unix/-m32, on a machine with AVX512.
I haven't tested with native-gdbserver/-m32 and native-extended-gdbserver.
> Since I happened to have access to a machine where this is
> a problem, I've also tested the patch, and the results there
> are good.
>
Great, thanks.
> This patch is really a giant step in the right direction.
> [-m32 gdbserver tests on AVX512 were rather useless before this]
So, how do we proceed? You'd like to see further testing at this point?
Thanks,
- Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [gdb/tdep] Fix avx512 -m32 support in gdbserver
2021-12-02 17:00 ` Tom de Vries
@ 2021-12-02 17:02 ` Keith Seitz
2021-12-02 17:18 ` Tom de Vries
0 siblings, 1 reply; 5+ messages in thread
From: Keith Seitz @ 2021-12-02 17:02 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 12/2/21 09:00, Tom de Vries wrote:
> On 12/2/21 5:43 PM, Keith Seitz wrote:
>> This patch is really a giant step in the right direction.
>> [-m32 gdbserver tests on AVX512 were rather useless before this]
>
> So, how do we proceed? You'd like to see further testing at this point?
Nope. I have high confidence the patch is ready to be pushed.
IANAM, but I encourage you to approve your own patch. :-)
Keith
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [gdb/tdep] Fix avx512 -m32 support in gdbserver
2021-12-02 17:02 ` Keith Seitz
@ 2021-12-02 17:18 ` Tom de Vries
0 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2021-12-02 17:18 UTC (permalink / raw)
To: Keith Seitz, gdb-patches
On 12/2/21 6:02 PM, Keith Seitz wrote:
> On 12/2/21 09:00, Tom de Vries wrote:
>> On 12/2/21 5:43 PM, Keith Seitz wrote:
>>> This patch is really a giant step in the right direction.
>>> [-m32 gdbserver tests on AVX512 were rather useless before this]
>>
>> So, how do we proceed? You'd like to see further testing at this point?
>
> Nope. I have high confidence the patch is ready to be pushed.
> IANAM, but I encourage you to approve your own patch. :-)
Thanks for the review then, and thanks again for the testing.
For the record, I didn't consider using the aforementioned gdbserver
boards because I got plenty of FAILs on unix/-m32 without the patch.
I'll commit shortly.
- Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-02 17:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30 9:11 [PATCH] [gdb/tdep] Fix avx512 -m32 support in gdbserver Tom de Vries
2021-12-02 16:43 ` Keith Seitz
2021-12-02 17:00 ` Tom de Vries
2021-12-02 17:02 ` Keith Seitz
2021-12-02 17:18 ` Tom de Vries
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).