From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 60D6F3858D28 for ; Mon, 28 Aug 2023 20:57:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 60D6F3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 37SKvJ3v022597 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Aug 2023 16:57:24 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 37SKvJ3v022597 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1693256244; bh=5b+CVBwoVSKaoTiQGjHhy1FnGpX1eVNKEyJn8G01R7A=; h=Date:Subject:To:References:From:In-Reply-To:From; b=pgzuoEES3942SMVugtK508vrZKgLsQpE4X8NIgzpBlPpMzJbPCbOpQ/5dJf89QWFe 5RL0lqouOZjL8DcRkpyMmDFYqMzIPUP4UzjVDeVB7ThjbWnIJDy9DhLjwFtj696ANg gwMDR4gnNM+0tuBQXaA6jcLQZhPsCHW+0lwKmF0w= Received: from [10.0.0.170] (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 7A0CF1E028; Mon, 28 Aug 2023 16:57:19 -0400 (EDT) Message-ID: Date: Mon, 28 Aug 2023 16:57:18 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 15/15] gdbserver: Simplify handling of ZMM registers. Content-Language: fr To: John Baldwin , gdb-patches@sourceware.org References: <20230714155151.21723-1-jhb@FreeBSD.org> <20230714155151.21723-16-jhb@FreeBSD.org> From: Simon Marchi In-Reply-To: <20230714155151.21723-16-jhb@FreeBSD.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 28 Aug 2023 20:57:19 +0000 X-Spam-Status: No, score=-3037.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 7/14/23 11:51, John Baldwin wrote: > - Reuse num_xmm_registers directly for the count of ZMM0-15 registers > as is already done for the YMM registers for AVX rather than using > a new variable that is always the same. > > - Replace 3 identical variables for the count of upper ZMM16-31 > registers with a single variable. Make use of this to merge > various loops working on the ZMM XSAVE region so that all of the > handling for the various sub-registers in this region are always > handled in a single loop. > > - While here, fix some bugs in i387_cache_to_xsave on where if > X86_XSTATE_ZMM was set on i386 (e.g. a 32-bit process on a 64-bit > kernel), the -1 register nums would wrap around and store the > value of GPRs in the XSAVE area. This should be harmless, but > is definitely odd. Instead, check num_zmm_high_registers directly > when checking X86_XSTATE_ZMM and skip the ZMM region handling > entirely if the register count is 0. > --- > gdbserver/i387-fp.cc | 132 +++++++++++++++---------------------------- > 1 file changed, 46 insertions(+), 86 deletions(-) > > diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc > index f53a6cfc477..91d3a0b8ca3 100644 > --- a/gdbserver/i387-fp.cc > +++ b/gdbserver/i387-fp.cc > @@ -267,12 +267,8 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf) > > /* 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; > + /* AVX512 adds 16 extra ZMM regs in Amd64 mode, but none in I386 mode.*/ > + int num_zmm_high_registers = amd64 ? 16 : 0; The naming of these variables is confusing because "high" can mean the high parts of the registers, or it can mean registers 16 to 31. I wish we could find different words to express both things. But at least your patch doesn't make things worse (it makes things better by eliminating some variables). Approved-By: Simon Marchi Simon