From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id eZefACvBomo/9DwAWB0awg (envelope-from ) for ; Thu, 10 Sep 2026 10:39:39 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=yahoo.de header.i=@yahoo.de header.a=rsa-sha256 header.s=s2048 header.b=VSThXI45; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id DFD971E09E; Thu, 10 Sep 2026 10:39:38 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [IPv6:2620:52:6:3111::32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 591C81E091 for ; Thu, 10 Sep 2026 10:39:34 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2566C4BAE7C6 for ; Thu, 10 Sep 2026 14:39:33 +0000 (GMT) Received: from sonic.asd.mail.yahoo.com (sonic-euwe4-0022.asd.mail.yahoo.com [34.2.86.21]) by sourceware.org (Postfix) with ESMTPS id E07F24BA2E0C for ; Thu, 10 Sep 2026 14:39:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E07F24BA2E0C Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=yahoo.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=yahoo.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E07F24BA2E0C Authentication-Results: sourceware.org; arc=none smtp.remote-ip=34.2.86.21 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789051160; cv=none; b=V1PZmLJoE16vAjtIZmd3Jnbid1lTqp/WhVty3g0ymOpVbnOUNuAUuLjPVCUoDYueY8XvdvagopfVmuSuO/RUeM+uq9kRsNqZ3kq03mAunCHnahnzBAjCFpbaIJH2ryyS9Zk2ctuoJLJErCDmZoHpJisVBm5uk5cko5hrhFHyUac= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789051160; c=relaxed/simple; bh=XvdT83hahgLmmYW4xaJ4GnOmd2FmJTPt3+qAQ1GfE8I=; h=DKIM-Signature:Date:From:To:Message-ID:Subject:MIME-Version; b=wgZotAuzWH81/5fYTTV6DBeuaovkyonKWRabZGXSHtHkOOFdLuCQOTdMyyB88N1CoxA1G/1Tu9bE1xFS4biKlJIiI2zl142J+CjaeUr7z5b4p808ZEzweelwNWHjKAHGsh0ZU/Ur+Dwf2I9LJUOtLpBBxZ8FAGg8KopWGnxnTEw= ARC-Authentication-Results: i=1; sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.de; s=s2048; t=1789051158; bh=XvdT83hahgLmmYW4xaJ4GnOmd2FmJTPt3+qAQ1GfE8I=; h=Date:From:To:Cc:In-Reply-To:References:Subject:From:Subject:Reply-To; b=VSThXI45WIda6IsjAK7DGo/Exx+hn3EwNIfqWcWHTuejNhlJ3vYYcfHcKRSIPH/EuVUvJoMe7CdmZ8RbjZ/3FjKRem0IT944MWhEoPWQgmiObW8hats9jn50gBKo8XHc4MokOgXvD40YDLEjhytgoZh4n6PoMJrF2I1GaEYehncNgVfuRXFLhq13wMdVjYiXRC2K4afPM7ZQ3hQMUhIJFkVsO+bxieD4PwCyPhQ/ZmZpSOBSWiC2ED9H9so4mOpjpMkcfVFd5PnOcK/3CxBvROpQjTG+pupLXbwQBNE4ON+e+A8oiEK8daXLSuiwwNorRK1hmMQ21fx3/5mqZ56UIg== X-SONIC-DKIM-SIGN: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1789051158; bh=eJ2PO4kejV4tCn1yF3Ymf+nakRiNUGi++Anpcgz/AvG=; h=X-Sonic-MF:Date:From:To:Subject:From:Subject; b=Gcnkzge6rDzyX5LwrFN/5FL9mAmqHAcRePYm68VvgmGJO+BKOdr0zj/XwDi/9ckUR7L9EqlJ6G1MIbb6nlSL2zfVj5agHlMzO9cabsVH5lgq+AAugZNSOLwB142WPPhN2WUKdWBdFfE54Gp1EWAqB/cAWS0Shs7GR4XBMVDHg/gd2TW50ocDYlY9JzBoPS9Q06yx4yjk1PZpLJdoX7BF9tJiL8Pd3kbhxHUn/qdmxXPPyDg/XuN1fXjkgkIgcgTyP0EOmVKtE+huZ4VmYwde7PhUnIPGZT/DZzUm/AgEmJrOP63wDARJYrcAUVPPlSWMS2RwCqwHkuCQ0hRgMYuiTQ== X-YMail-OSG: t9VHguUVM1nv4F7n60GsdeHK019kuWRD.Jx0pEWei1txFfMatOMp8M9BsPNWNmU BN1pm7kh4FO1YqJCjC4jcfviyYNy8sKMutVgK_8tIZReeajH.tzXz5lLKrB712ZKoVLlAqOERc1w abm6dQ3mv4TZNo1at9Y2vwKsR.GCp_1lOp_5nRhkZ5DKZ1gsTxTPebv4wrZuN6dYSy9bYyC3cQi6 r6eLGLa5W93uOGLLfbTzQXIFBlVSsm90TXb_SVyP.C1Jo8mQ3nTngJfs8cBZn9ZXEabMWd9ydjRK c5Td4T1fyfghtMw0tt4YCWlFvJmi8JDcXNCZChR5759dGHUmmvBejNjVKZ_a4YPAwOy9bnzTG5B_ nqU7ApA51HuCuwUeXa2q.l2aPq1wgdQQBxZQhPo_QwW3_5q89DSL87u710fTM.z0g7sKoRIoZAi3 IoLMHE6oYTnl.RM1rfcPKQgs5kATlMdWeU6ZqeAb6CqUwelWLk8At9ZwPbJoRCGnjtr3ncSSWMLS ssNiSJwtkwZhSlZYi7iwxiaJX4ZBm9g17JJogrbSlCF_xceC5w3.5KVijAGxbf7QhqgER9GYMrvI uEsMjWa.Kfu.8LX_Lz_gSHu59JLUgldqOKDSaPGbQM3tmii3eFN5GF2rcSqI7ysOM3gkPDoyjjY3 WRbidK1C1ZpSrtsRcIGVvkJvckz1iHY98VQoxr9hWMrwGJWOouxmLeyzb9EboNGYZ4pJKKvsoQw6 OuNouXbZzjht3npCuYfPo8d2VWaEYsffkPioRlmUqlPfZVr_59GCrhRU_YHdjRYR_mvZi.zWsKeQ skGbzpq4aSILK0X_8P8wPbslJFvZ45dnf.58BFtYBDjYVZFj4OOd4B5JQBAsx4cRxx_qyJrbJDWd wOgEc5lGlxKDovoo5lw5CwnTmUPsJwsXeXzv5eyin77cMu_COZdlSETbjiorM3V.zHHWNUvsxheN FEY6FS46io.xxVs4mvcazB7.oboFDfqZxglfF9t3KalN9eBnhuJM_Zou9zvjYMcvcszJzWaYkeiF Ezb.1ewYpwH481HswaWpYXXrfz6eZEvg7KXLTXBLnCIX3BNpVppN7s4XU2pHueLvO2sK8UalgMu6 .khJqM_i9_4D2uYRpIkHo4kREW1cnpHVfUAQt__ebnt9X5SSfFmbJKY_gIZz8LEcYiM6XW8FgXAI QICxScrgoi1EKUbAE1EIUk6bC.MCB9W49GlvTDGeymEf0YguL4TZ72joG156f6JQXlSQvaLgvnbi rTZIpY6CTAQJlQd3uPV6aWzNlHJyCdFgSfEEVzGpm8eVKfx7aXFld.paeRdRooTRrf5T0YX4g7O_ VfR4QkOONn5WAMItXfjapdydI.FVYu.63sW_ZCMAEKwr2EAqmUygGgrhZGEepviMlZLT6pLKhEej j8bAi1pvn0vsqGrl6J9jDThha11ZkwLB0u2uwkTvvpk54l67H_9jLnbNUMqwvbIVUc4OG9BUAWFz rSWzJ6kLI2xJQTSVzAdNV8gnMLHrtIydP8MECj.GO6SvcQOZhGZONRgN7LrY.eMwIYecIgTJ40Bf x1VYU8gIeRtC.ei4rFuR3zP4hWUaZRGCU3Nad1hXX_vJZF9EsvgRnsiPwsNkHvNhVGMZ3nM.l9W9 _Z0Rb4LgEYJYXZPTpkdDv6M9yeAL6xqscAac2FKUi7FZqSZqWLPG6sopCIUkFUFNAA2uVgB.Zxco Q1oTqDdHs59aANli6ruDC8Sl2YsuvGqw0m0_n95KJXAh_tgGMf9apQbz0e5HVmktTdGMPY1uZYyq zUPMVnJo4yWHqIJa9SRf.8WXUCulrXAbrwdIAfSDF1qP.3B25mu7sKiRje8IGW8fjTG9X82SwWVk esuP4yX085L3tSG7X5bQ7us.QqGhhogkiWOF8Hsp5xTOj6mK4n5SzbWpOE7v44q.1Foq0mSfUfpM p0b8l10u1hGz2Tl_Pdw3EQ9yslkqRv60zxadU9C3ne4kCct8GXF7RZMLWJf5MPXeU3eX8n54I5nr 69frmNGbhlw8MahyYgMcYVTWpYC65iyG9WThqXGS0PUh4mMqWsFAI6LfnuwmZKYZQHZg05_IXE4_ fKQ-- X-Sonic-MF: X-Sonic-ID: 5993022b-3cec-4683-b7da-8e554b1f315f Received: from sonic.gate.mail.ne1.yahoo.com by mail-asdoutdeli-p-cin-euwe4-prod-sonicconsumer-svc-102 with HTTP; Thu, 10 Sep 2026 14:39:18 +0000 Date: Thu, 10 Sep 2026 14:39:16 +0000 (UTC) From: Hannes Domani To: "Rohr, Stephan" , "gdb-patches@sourceware.org" , "Joos, Christina" Cc: Tom Tromey Message-ID: <1124964361.369140.1789051156372@mail.yahoo.com> In-Reply-To: References: <20260829145823.1034821-1-ssbssa@yahoo.de> <20260829145823.1034821-8-ssbssa@yahoo.de> Subject: Re: [PATCH v3 8/8] Windows gdb: Implement AVX-512 register support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Mailer: WebService/1.1.26460 YMailCLDNorrin X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Am Donnerstag, 10. September 2026 um 13:33:20 MESZ hat Joos, Christina Folgendes geschrieben: > Hi Hannes, >=C2=A0 > I saw that Stephan already reviewed this (thanks!). > I added my remarks on top, see below. >=C2=A0 > > -----Original Message----- > > From: Rohr, Stephan > > Sent: Dienstag, 8. September 2026 15:05 > > To: Hannes Domani ; gdb-patches@sourceware.org; gdb- > > patches@sourceware.org > > Cc: Joos, Christina ; Tom Tromey > > > > Subject: RE: [PATCH v3 7/8] Windows gdb: Implement AVX register support > > > > Hi Hannes, > > > > please see some feedback inlined below. > > > > Let me know if you have any questions. > > > > Thanks > > Stephan > > > > > -----Original Message----- > > > From: Hannes Domani > > > Sent: Saturday, 29 August 2026 16:49 > > > To: gdb-patches@sourceware.org > > > Subject: [PATCH v3 7/8] Windows gdb: Implement AVX register support > > > > > > This adds support for the Intel AVX registers on Windows. > > > It enables accessing registers $ymm0 - $ymm15 where they are availabl= e. > > > > > > After this patch gdb.arch/i386-avx.exp passes on windows. > > > --- > > > v3: > > >=C2=A0 - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts > > > --- > > >=C2=A0 gdb/NEWS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 |=C2=A0 3 ++ > > >=C2=A0 gdb/nat/windows-nat.c=C2=A0 =C2=A0 =C2=A0 |=C2=A0 2 +- > > >=C2=A0 gdb/x86-windows-nat.c=C2=A0 =C2=A0 =C2=A0 | 67 > > > +++++++++++++++++++++++++++++++++++-- > > >=C2=A0 gdbserver/win32-i386-low.cc | 61 +++++++++++++++++++++++++++++-= --- > > >=C2=A0 gdbserver/win32-low.cc=C2=A0 =C2=A0 =C2=A0 | 15 ++++++--- > > >=C2=A0 5 files changed, 133 insertions(+), 15 deletions(-) > > > > > > diff --git a/gdb/NEWS b/gdb/NEWS > > > index 10c182067f9..f7effc822e9 100644 > > > --- a/gdb/NEWS > > > +++ b/gdb/NEWS > > > @@ -118,6 +118,9 @@ > > >=C2=A0 =C2=A0 intent to remove it in a future release. > > >=C2=A0 =C2=A0 The s390 64-bit target (s390x-*) remains supported. > > > > > > +* Support for Intel AVX registers on Windows. > > > +=C2=A0 Support displaying and modifying Intel AVX registers $ymm0 - = $ymm31. > > > + > > > > I think this should be registers $ymm0 - $ymm15 ? >=C2=A0 > Yes, I agree with Stephan's feedback here. > The AVX state only comprises only YMM0=E2=80=93YMM15 for 64 bit. > For 32-bit mode, it's YMM0- YMM7 only. > See the docs added in commit "Add org.gnu.gdb.i386.avx." Yes, I missed this when I split AVX/AVX-512. > > >=C2=A0 * Configure changes > > > > > >=C2=A0 ** --with-babeltrace has been removed.=C2=A0 The babeltrace lib= rary was > > > diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c index > > > 8930536f3ba..c9a21d7c41f 100644 > > > --- a/gdb/nat/windows-nat.c > > > +++ b/gdb/nat/windows-nat.c > > > @@ -1339,7 +1339,7 @@ initialize_loadable () > > >=C2=A0 =C2=A0 =C2=A0 { > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Available XState features masked with i= mplemented features.=C2=A0 */ > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 xstate_features =3D (GetEnabledXStateFeatu= res () > > > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 & X86_XSTATE_SSE_MASK); > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 & X86_XSTATE_AVX_MASK); > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* The extended XState functions are only = needed if the available > > >=C2=A0 =C2=A0 =C2=A0 features exceed SSE.=C2=A0 */ > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((xstate_features & ~X86_XSTATE_SSE_MAS= K) =3D=3D 0) diff --git > > > a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c index > > > 3af5ef4dae0..1cefe6171be 100644 > > > --- a/gdb/x86-windows-nat.c > > > +++ b/gdb/x86-windows-nat.c > > > @@ -27,6 +27,9 @@ > > > > > >=C2=A0 #include "i386-tdep.h" > > >=C2=A0 #include "i387-tdep.h" > > > +#ifdef __x86_64__ > > > +#include "amd64-tdep.h" > > > +#endif > > > > > >=C2=A0 using namespace windows_nat; > > > > > > @@ -70,6 +73,8 @@ struct x86_windows_nat_target final : public > > > x86_nat_target > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 windows_thread= _info *th, int r) override; > > > > > >=C2=A0 =C2=A0 bool is_sw_breakpoint (const EXCEPTION_RECORD *er) const= override; > > > + > > > +=C2=A0 const struct target_desc *read_description () override; > > >=C2=A0 }; > > > > > >=C2=A0 /* The current process.=C2=A0 */ > > > @@ -109,7 +114,31 @@ x86_windows_per_inferior::fill_thread_context > > > (windows_thread_info *th) > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (context->ContextFlags =3D=3D 0) > > >=C2=A0 =C2=A0 =C2=A0 { > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 context->ContextFlags =3D WindowsContext::all; > > > +=C2=A0 =C2=A0 =C2=A0 if (xstate_features !=3D 0) > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 context->ContextFlags |=3D CONTEX= T_XSTATE_FLAG; > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 set_xstate_features_mask (context= , xstate_features); > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 } > > > > We have the same code in "i386_get_thread_context" in "win32-i386-low.c= c". > > Make a shared function in gdb/nat/windows-nat.h? > > > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 CHECK (get_thread_context (th->h, context)= ); > > > + > > > +=C2=A0 =C2=A0 =C2=A0 if (xstate_features !=3D 0) > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DWORD64 features =3D 0; > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 CHECK (get_xstate_features_mask (= context, &features)); > > > > Should this be changed to sth. like > > > >=C2=A0 if (!get_xstate_features_mask (context, &features)) > >=C2=A0 =C2=A0 { > >=C2=A0 =C2=A0 =C2=A0 warning (..) > >=C2=A0 =C2=A0 =C2=A0 return; > >=C2=A0 =C2=A0 } > > > > The call of "CHECK" only prints a message but doesn't error out.=C2=A0 = If this call fails > > we may still have features =3D=3D 0.=C2=A0 This implies "zeroed_feature= s =3D=3D > > xstate_features".=C2=A0 With this, the loop clears all features. > > IIUC, this would clear the AVX registers on the next call of "SetThread= Context". > > > > Also refer to the implementation in gdbserver/win32-i386-low.cc: > > > >=C2=A0 =C2=A0 =C2=A0 DWORD64 features =3D 0; > >=C2=A0 =C2=A0 =C2=A0 if (xstate_features !=3D 0 > >=C2=A0 =C2=A0 =C2=A0 =C2=A0&& get_xstate_features_mask (context, &featur= es)) > >=C2=A0 =C2=A0 =C2=A0{ > > > > I think it makes sense to unify those as the rest of the code is basica= lly identical. > > Put shared function into gdb/nat/windows-nat.h?=C2=A0 This keeps the co= de > > consistent. I will try that. > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DWORD64 zeroed_features =3D xstat= e_features & ~features; > > > + > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for (int f =3D X86_XSTATE_AVX_ID;= f <=3D X86_XSTATE_CET_U_ID; f++) >=C2=A0 > In addition to Stephan's feedback for this code area: > Since we decided to not add CET, and this patch is even for AVX only for = now only, do we need a loop at this point already? >=C2=A0 > For the follow up AVX-512 patch I think we can stop at the highest suppor= ted feature (AVX-512) in windows, can't we? I would prefer it if we could keep it a loop, even if it's only X86_XSTATE_= AVX_ID in the AVX patch. And yes, I forgot to change it to X86_XSTATE_ZMM_ID when I removed the CET = stuff. > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DWORD64 flag =3D 1ULL << f; > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((zeroed_features & flag) !=3D= 0) > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DWORD size =3D 0; > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 void *loc =3D locat= e_xstate_feature (context, f, &size); > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (loc !=3D nullpt= r && size > 0) > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 memset (loc, 0, size); > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 } > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 } > > >=C2=A0 =C2=A0 =C2=A0 } > > >=C2=A0 =C2=A0 =C2=A0 }); > > >=C2=A0 } > > > @@ -198,6 +227,14 @@ > > > x86_windows_nat_target::thread_context_continue (windows_thread_info > > > *th, > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (GetExitCodeThread (th->h, &ec) > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 && ec =3D=3D STILL_ACTIVE) > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DWORD debug_registers =3D > > > WindowsContext::debug; > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (xstate_features !=3D 0 > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 && (context->ContextFlags & ~debu= g_registers) !=3D 0) > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 context->ContextFlags |=3D CONTEX= T_XSTATE_FLAG; > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 set_xstate_features_mask (context= , xstate_features); > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 } > > > + > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BOOL status =3D set_thread_c= ontext (th->h, context); > > > > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!killed) > > > @@ -227,7 +264,7 @@ x86_windows_nat_target::thread_context_step > > > (windows_thread_info *th, > > > > > >=C2=A0 template > > >=C2=A0 static char * > > > -get_context_reg_ptr (Context *context, int r) > > > +get_context_reg_ptr (Context *context, int r, i386_gdbarch_tdep > > > +*tdep) > > >=C2=A0 { > > >=C2=A0 =C2=A0 const int *mappings; > > >=C2=A0 =C2=A0 int mappings_count; > > > @@ -247,6 +284,13 @@ get_context_reg_ptr (Context *context, int r) > > >=C2=A0 =C2=A0 char *context_offset; > > >=C2=A0 =C2=A0 if (r < mappings_count) > > >=C2=A0 =C2=A0 =C2=A0 context_offset =3D (char *) context + mappings[r]= ; > > > +=C2=A0 else if (I387_YMM0H_REGNUM (tdep) > 0 && r >=3D > > I387_YMM0H_REGNUM > > > (tdep) > > > +=C2=A0 =C2=A0 =C2=A0 && r < I387_YMMENDH_REGNUM (tdep)) > > > > The implementation on gdbserver side guards against > > > >=C2=A0 xstate_features & X86_XSTATE_AVX) !=3D 0 > > > > I wonder if the same guard would be helpful here, too.=C2=A0 I understa= nd the > > register number is initialized to -1, so this should not fire.=C2=A0 I'= m not sure if it is > > possible to have $ymm0 register number > 0 w/o xstate support, e.g., if= the > > target description is read from file, see "target_find_description"? On gdbserver side I needed the guard because there is no register number I could check. I didn't think it was necessary on gdb side as well, but if you prefer it like this, I will add it. > > > +=C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 context_offset =3D (char *) locate_xstate_featu= re > > > +=C2=A0 =C2=A0 (context, X86_XSTATE_AVX_ID, NULL); >=C2=A0 > We prefer to use nullptr. Right. > > > +=C2=A0 =C2=A0 =C2=A0 context_offset +=3D 16 * (r - I387_YMM0H_REGNUM= (tdep)); > > > +=C2=A0 =C2=A0 } > > >=C2=A0 =C2=A0 else > > >=C2=A0 =C2=A0 =C2=A0 gdb_assert_not_reached ("invalid register number = %d", r); > > > > > > @@ -267,7 +311,7 @@ x86_windows_nat_target::fetch_one_register (struc= t > > > regcache *regcache, > > >=C2=A0 =C2=A0 char *context_offset > > >=C2=A0 =C2=A0 =C2=A0 =3D x86_windows_process.with_context (th, [&] (au= to *context) > > >=C2=A0 =C2=A0 =C2=A0 { > > > -=C2=A0 =C2=A0 =C2=A0 return get_context_reg_ptr (context, r); > > > +=C2=A0 =C2=A0 =C2=A0 return get_context_reg_ptr (context, r, tdep); > > >=C2=A0 =C2=A0 =C2=A0 }); > > > > > >=C2=A0 =C2=A0 gdb_assert (!gdbarch_read_pc_p (gdbarch)); @@ -333,7 +37= 7,7 @@ > > > x86_windows_nat_target::store_one_register (const struct regcache > > > *regcache, > > >=C2=A0 =C2=A0 =C2=A0 =3D x86_windows_process.with_context (th, [&] (au= to *context) > > >=C2=A0 =C2=A0 =C2=A0 { > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_assert (context->ContextFlags !=3D 0); > > > -=C2=A0 =C2=A0 =C2=A0 return get_context_reg_ptr (context, r); > > > +=C2=A0 =C2=A0 =C2=A0 return get_context_reg_ptr (context, r, tdep); > > >=C2=A0 =C2=A0 =C2=A0 }); > > > > > >=C2=A0 =C2=A0 /* GDB treats some registers as 32-bit, where they are i= n fact only > > > @@ -368,6 +412,23 @@ x86_windows_nat_target::is_sw_breakpoint (const > > > EXCEPTION_RECORD *er) const > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 || er->ExceptionCode =3D=3D STATUS_WX86_BR= EAKPOINT);=C2=A0 } > > > > > > +const struct target_desc * > > > +x86_windows_nat_target::read_description () { > > > +=C2=A0 if (inferior_ptid =3D=3D null_ptid) > > > +=C2=A0 =C2=A0 return this->beneath ()->read_description (); > > > + > > > +=C2=A0 if (xstate_features =3D=3D 0) > > > +=C2=A0 =C2=A0 return nullptr; > > > + > > > +#ifdef __x86_64__ > > > +=C2=A0 if (!x86_windows_process.wow64_process) > > > +=C2=A0 =C2=A0 return amd64_target_description (xstate_features, fals= e); > > > +=C2=A0 else > > > +#endif > > > +=C2=A0 =C2=A0 return i386_target_description (xstate_features, false= ); } > > > + > > >=C2=A0 /* Hardware watchpoint support, adapted from go32-nat.c code.= =C2=A0 */ > > > > > >=C2=A0 /* Pass the address ADDR to the inferior in the I'th debug regi= ster. > > > diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.c= c > > > index b77f6adc6ed..a7e83c0239c 100644 > > > --- a/gdbserver/win32-i386-low.cc > > > +++ b/gdbserver/win32-i386-low.cc > > > @@ -253,6 +253,11 @@ i386_get_thread_context (windows_thread_info > > > *th) > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = | WindowsContext::floating > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = | WindowsContext::debug > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = | extended_registers); > > > +=C2=A0 =C2=A0 =C2=A0 if (xstate_features !=3D 0) > > > +=C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 context->ContextFlags |=3D CONTEXT_XSTATE_FLAG; > > > +=C2=A0 =C2=A0 =C2=A0 set_xstate_features_mask (context, xstate_featu= res); > > > +=C2=A0 =C2=A0 } > > > > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 BOOL ret =3D get_thread_context (th->h, co= ntext); > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!ret) > > > @@ -267,6 +272,24 @@ i386_get_thread_context (windows_thread_info > > > *th) > > > > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 error (_("GetThreadContext failure %ld\n")= , (long) e); > > >=C2=A0 =C2=A0 =C2=A0 } > > > + > > > +=C2=A0 =C2=A0 =C2=A0 DWORD64 features =3D 0; > > > +=C2=A0 =C2=A0 =C2=A0 if (xstate_features !=3D 0 > > > +=C2=A0 =C2=A0 =C2=A0 && get_xstate_features_mask (context, &features= )) > > > +=C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 DWORD64 zeroed_features =3D xstate_features & ~= features; > > > +=C2=A0 =C2=A0 =C2=A0 for (int f =3D X86_XSTATE_AVX_ID; f <=3D X86_XS= TATE_CET_U_ID; f++) > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DWORD64 flag =3D 1ULL << f; > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((zeroed_features & flag) !=3D= 0) > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DWORD size =3D 0; > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 void *loc =3D locate_xstate_featu= re (context, f, &size); > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (loc !=3D nullptr && size > 0) > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 memset (loc, 0, size); > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 } > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 } > > > +=C2=A0 =C2=A0 } > > >=C2=A0 =C2=A0 =C2=A0 }); > > >=C2=A0 } > > > > > > @@ -292,6 +315,17 @@ i386_prepare_to_resume (windows_thread_info > > > *th) > > > > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 th->debug_registers_changed =3D false; > > >=C2=A0 =C2=A0 =C2=A0 } > > > + > > > +=C2=A0 windows_process.with_context (th, [&] (auto *context) > > > +=C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 DWORD debug_registers =3D WindowsContext::debug; > > > +=C2=A0 =C2=A0 =C2=A0 if (xstate_features !=3D 0 > > > +=C2=A0 =C2=A0 =C2=A0 && (context->ContextFlags & ~debug_registers) != =3D 0) > > > +=C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 context->ContextFlags |=3D CONTEXT_XSTATE_FLAG; > > > +=C2=A0 =C2=A0 =C2=A0 set_xstate_features_mask (context, xstate_featu= res); > > > +=C2=A0 =C2=A0 } > > > +=C2=A0 =C2=A0 }); > > >=C2=A0 } > > > > > >=C2=A0 static void > > > @@ -477,7 +511,7 @@ is_segment_register (int r) > > > > > >=C2=A0 template > > >=C2=A0 static char * > > > -get_context_reg_ptr (Context *context, int r) > > > +get_context_reg_ptr (Context *context, int r, const target_desc > > > +*tdesc) > > >=C2=A0 { > > >=C2=A0 =C2=A0 const int *mappings; > > >=C2=A0 =C2=A0 int mappings_count; > > > @@ -494,9 +528,21 @@ get_context_reg_ptr (Context *context, int r) > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 mappings_count =3D sizeof (i386_mappings) = / sizeof (i386_mappings[0]); > > >=C2=A0 =C2=A0 =C2=A0 } > > > > > > +=C2=A0 bool amd64 =3D register_size (tdesc, 0) =3D=3D 8; > > > > There is already an " if (!windows_process.wow64_process)" a few lines = above. > > Wouldn't it make sense to move the "bool amd64" in the ifdef blocks and= assign > > accordingly? Yes, I agree. > > > +=C2=A0 int ymm0h_regnum; > > > +=C2=A0 const int num_xmm_registers =3D amd64 ? 16 : 8; > > > + > > >=C2=A0 =C2=A0 char *context_offset; > > >=C2=A0 =C2=A0 if (r < mappings_count) > > >=C2=A0 =C2=A0 =C2=A0 context_offset =3D (char *) context + mappings[r]= ; > > > +=C2=A0 else if ((xstate_features & X86_XSTATE_AVX) !=3D 0 > > > +=C2=A0 =C2=A0 =C2=A0 && r >=3D (ymm0h_regnum =3D find_regno (tdesc, = "ymm0h")) > > > +=C2=A0 =C2=A0 =C2=A0 && r < ymm0h_regnum + num_xmm_registers) > > > +=C2=A0 =C2=A0 { > > > +=C2=A0 =C2=A0 =C2=A0 context_offset =3D (char *) locate_xstate_featu= re > > > +=C2=A0 =C2=A0 (context, X86_XSTATE_AVX_ID, NULL); > > > +=C2=A0 =C2=A0 =C2=A0 context_offset +=3D 16 * (r - ymm0h_regnum); > > > +=C2=A0 =C2=A0 } > > >=C2=A0 =C2=A0 else > > >=C2=A0 =C2=A0 =C2=A0 gdb_assert_not_reached ("invalid register number = %d", r); > > > > > > @@ -510,7 +556,7 @@ i386_fetch_inferior_register (struct regcache > > > *regcache,=C2=A0 { > > >=C2=A0 =C2=A0 char *context_offset =3D windows_process.with_context (t= h, [&] (auto > > > *context) > > >=C2=A0 =C2=A0 =C2=A0 { > > > -=C2=A0 =C2=A0 =C2=A0 return get_context_reg_ptr (context, r); > > > +=C2=A0 =C2=A0 =C2=A0 return get_context_reg_ptr (context, r, regcach= e->tdesc); > > >=C2=A0 =C2=A0 =C2=A0 }); > > > > > >=C2=A0 =C2=A0 /* GDB treats some registers as 32-bit, where they are i= n fact only > > > @@ -538,7 +584,7 @@ i386_store_inferior_register (struct regcache > > > *regcache,=C2=A0 { > > >=C2=A0 =C2=A0 char *context_offset =3D windows_process.with_context (t= h, [&] (auto > > > *context) > > >=C2=A0 =C2=A0 =C2=A0 { > > > -=C2=A0 =C2=A0 =C2=A0 return get_context_reg_ptr (context, r); > > > +=C2=A0 =C2=A0 =C2=A0 return get_context_reg_ptr (context, r, regcach= e->tdesc); > > >=C2=A0 =C2=A0 =C2=A0 }); > > > > > >=C2=A0 =C2=A0 /* GDB treats some registers as 32-bit, where they are i= n fact only > > > @@ -571,14 +617,17 @@ i386_arch_setup (void)=C2=A0 { > > >=C2=A0 target_desc_up tdesc; > > > > > > +=C2=A0 DWORD64 xcr0 =3D xstate_features; > > > +=C2=A0 if (xcr0 =3D=3D 0) > > > +=C2=A0 =C2=A0 xcr0 =3D X86_XSTATE_SSE_MASK; > > > + > > >=C2=A0 #ifdef __x86_64__ > > > -=C2=A0 tdesc =3D amd64_create_target_description (X86_XSTATE_SSE_MAS= K, false, > > > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 false, false); > > > +=C2=A0 tdesc =3D amd64_create_target_description (xcr0, false, false= , > > > + false); > > >=C2=A0 =C2=A0 init_target_desc (tdesc.get (), amd64_expedite_regs, WIN= DOWS_OSABI); > > >=C2=A0 =C2=A0 win32_tdesc =3D std::move (tdesc); > > >=C2=A0 #endif > > > > > > -=C2=A0 tdesc =3D i386_create_target_description (X86_XSTATE_SSE_MASK= , false, > > > false); > > > +=C2=A0 tdesc =3D i386_create_target_description (xcr0, false, false)= ; > > >=C2=A0 =C2=A0 init_target_desc (tdesc.get (), i386_expedite_regs, WIND= OWS_OSABI); > > > #ifdef __x86_64__ > > >=C2=A0 =C2=A0 wow64_win32_tdesc =3D std::move (tdesc); diff --git > > > a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index > > > 7629beca213..5ccdc89a7ef 100644 > > > --- a/gdbserver/win32-low.cc > > > +++ b/gdbserver/win32-low.cc > > > @@ -33,6 +33,7 @@ > > >=C2=A0 #include > > >=C2=A0 #include "gdbsupport/gdb_tilde_expand.h" > > >=C2=A0 #include "gdbsupport/common-inferior.h" > > > +#include "tdesc.h" > > > > > >=C2=A0 using namespace windows_nat; > > > > > > @@ -426,8 +427,9 @@ child_fetch_inferior_registers (struct regcache > > > *regcache, int r) > > >=C2=A0 =C2=A0 int regno; > > >=C2=A0 =C2=A0 windows_thread_info *th =3D windows_process.find_thread > > > (current_thread- > > > >id); > > >=C2=A0 =C2=A0 win32_require_context (th); > > > -=C2=A0 if (r =3D=3D -1 || r > NUM_REGS) > > > -=C2=A0 =C2=A0 child_fetch_inferior_registers (regcache, NUM_REGS); > > > > IIUC this was the only use of the NUM_REGS define.=C2=A0 We can remove = it. >=C2=A0 > Yes, I agree. Would you mind explaining why we don't need this check anym= ore, too ? :) > I don't understand it yet, unfortunately. Was it necessary before or is t= his due to AVX? I don't think the check was necessary before AVX. But with AVX it doesn't work, because NUM_REGS is the number of fix registe= rs, so for any AVX register it would go to the 'else' part. > > Same for " i386_win32_num_regs (void)" and "aarch64_win32_num_regs ()". > > This allows removing the num_regs hook in win32_target_ops. >=C2=A0 > > > +=C2=A0 if (r =3D=3D -1) > > > +=C2=A0 =C2=A0 child_fetch_inferior_registers (regcache, > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 regcache->tdesc->reg_defs.size ()); > > >=C2=A0 =C2=A0 else > > >=C2=A0 =C2=A0 =C2=A0 for (regno =3D 0; regno < r; regno++) > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 (*the_low_target.fetch_inferior_register) = (regcache, th, > > > regno); @@ -441,8 +443,9 @@ child_store_inferior_registers (struct > > > regcache *regcache, int r) > > >=C2=A0 =C2=A0 int regno; > > >=C2=A0 =C2=A0 windows_thread_info *th =3D windows_process.find_thread > > > (current_thread- > > > >id); > > >=C2=A0 =C2=A0 win32_require_context (th); > > > -=C2=A0 if (r =3D=3D -1 || r =3D=3D 0 || r > NUM_REGS) > > > -=C2=A0 =C2=A0 child_store_inferior_registers (regcache, NUM_REGS); > > > +=C2=A0 if (r =3D=3D -1) > > > +=C2=A0 =C2=A0 child_store_inferior_registers (regcache, > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 regcache->tdesc->reg_defs.size ()); >=C2=A0 > I assume removing r =3D=3D 0 is not related to AVX register support and j= ust a cleanup, right? > Could we make it a separate cleanup patch (with reason)? >=C2=A0 > I know it seems like a super tiny nit, but it would help to understand wh= y that kind of > refactoring/cleanup is necessary (e.g. due to AVX or not). I removed r =3D=3D 0 because it wasn't there in child_fetch_inferior_regist= ers either, and I didn't like the this inconsistency. > > >=C2=A0 =C2=A0 else > > >=C2=A0 =C2=A0 =C2=A0 for (regno =3D 0; regno < r; regno++) > > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 (*the_low_target.store_inferior_register) = (regcache, th, > > > regno); @@ -1349,7 +1352,9 @@ void=C2=A0 initialize_low (void)=C2=A0 = { > > >=C2=A0 =C2=A0 set_target_ops (&the_win32_target); > > > -=C2=A0 the_low_target.arch_setup (); > > > > > >=C2=A0 =C2=A0 initialize_loadable (); > > > +=C2=A0 /* Has to be done after initialize_loadable, because it uses = the xstate > > > +=C2=A0 =C2=A0 functions if available.=C2=A0 */ > > > +=C2=A0 the_low_target.arch_setup (); > > >=C2=A0 } > > > -- > > > 2.54.0 >=C2=A0 > Christina Thanks Hannes