public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: John Baldwin <jhb@FreeBSD.org>
To: "George, Jini Susan" <JiniSusan.George@amd.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH v3 00/13] Handle variable XSAVE layouts
Date: Wed, 18 May 2022 14:48:41 -0700	[thread overview]
Message-ID: <9aa0af42-2405-60b6-dc4e-4333c3239314@FreeBSD.org> (raw)
In-Reply-To: <BY5PR12MB496599C1B1A6F8F3B570589F90C99@BY5PR12MB4965.namprd12.prod.outlook.com>

On 5/10/22 3:48 AM, George, Jini Susan wrote:
> [Public]
> 
> Thanks again for doing this, John, Felix and Aleksandar. I went through the changes and tested on a few AMD CPUs of differing xsave layouts. As of now these changes seem to cover the current AMD CPU implementations. Going forward, if additional AMD specific layouts have to be added, we would do it.
> 
> The changes look good to me (I am no approver, though) for the most part. A few minor comments:
> 
> Nit: in gdb/i386-tdep.h, it might be good to add a comment for the following line  (line # 148) for consistency with the rest of the fields.
>    x86_xsave_layout xsave_layout;

Done.

> Nit: You might want to modify the comments preceding the modified  xsave_*_offset[] definitions in gdb/i387-tdep.c to reflect that the offset to the locations are at (tdep)->xsave_layout.*_offset + xsave_*_offset[REGNUM].

Ok, I have done so locally though tried to describe this a bit differently.
For example:

/* At xsave_avxh_offset[REGNUM] you'll find the relative offset within
    the AVX region of the XSAVE extended state where the upper 128bits
    of GDB register YMM0 + REGNUM is stored.  */

> Nit: Line 771 in gdb/i387-tdep.c. Might make sense to remove the "HI16_ZMM_area +" part.

Done.

Thanks!

-- 
John Baldwin

  reply	other threads:[~2022-05-18 21:48 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-03 21:05 John Baldwin
2022-05-03 21:05 ` [PATCH v3 01/13] x86: Add an x86_xsave_layout structure to handle " John Baldwin
2022-05-03 21:05 ` [PATCH v3 02/13] core: Support fetching TARGET_OBJECT_X86_XSAVE_LAYOUT from architectures John Baldwin
2022-05-03 21:05 ` [PATCH v3 03/13] nat/x86-cpuid.h: Add x86_cpuid_count wrapper around __get_cpuid_count John Baldwin
2022-05-03 21:05 ` [PATCH v3 04/13] x86 nat: Add helper functions to save the XSAVE layout for the host John Baldwin
2022-05-03 21:05 ` [PATCH v3 05/13] gdb: Update x86 FreeBSD architectures to support XSAVE layouts John Baldwin
2022-05-03 21:05 ` [PATCH v3 06/13] gdb: Support XSAVE layouts for the current host in the FreeBSD x86 targets John Baldwin
2022-05-03 21:05 ` [PATCH v3 07/13] gdb: Update x86 Linux architectures to support XSAVE layouts John Baldwin
2022-05-03 21:05 ` [PATCH v3 08/13] gdb: Support XSAVE layouts for the current host in the Linux x86 targets John Baldwin
2022-05-03 21:05 ` [PATCH v3 09/13] gdb: Use x86_xstate_layout to parse the XSAVE extended state area John Baldwin
2022-05-03 21:05 ` [PATCH v3 10/13] gdbserver: Add a function to set the XSAVE mask and size John Baldwin
2022-05-03 21:05 ` [PATCH v3 11/13] gdbserver: Refactor the legacy region within the xsave struct John Baldwin
2022-05-03 21:05 ` [PATCH v3 12/13] gdbserver: Read offsets of the XSAVE extended region via CPUID John Baldwin
2022-05-03 21:05 ` [PATCH v3 13/13] x86: Remove X86_XSTATE_SIZE and related constants John Baldwin
2022-05-10 10:48 ` [PATCH v3 00/13] Handle variable XSAVE layouts George, Jini Susan
2022-05-18 21:48   ` John Baldwin [this message]
2022-05-18 21:53 ` John Baldwin
2022-05-19 15:42   ` Willgerodt, Felix
2022-05-19 16:05     ` John Baldwin
2022-05-20 14:53       ` Willgerodt, Felix

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9aa0af42-2405-60b6-dc4e-4333c3239314@FreeBSD.org \
    --to=jhb@freebsd.org \
    --cc=JiniSusan.George@amd.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).