From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7782 invoked by alias); 25 Nov 2013 16:50:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 7768 invoked by uid 89); 25 Nov 2013 16:50:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.1 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mga09.intel.com Received: from Unknown (HELO mga09.intel.com) (134.134.136.24) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 25 Nov 2013 16:50:21 +0000 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 25 Nov 2013 08:46:16 -0800 X-ExtLoop1: 1 Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by orsmga001.jf.intel.com with ESMTP; 25 Nov 2013 08:49:52 -0800 Received: from irsmsx152.ger.corp.intel.com (163.33.192.66) by IRSMSX103.ger.corp.intel.com (163.33.3.157) with Microsoft SMTP Server (TLS) id 14.3.123.3; Mon, 25 Nov 2013 16:48:55 +0000 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.135]) by IRSMSX152.ger.corp.intel.com ([169.254.6.169]) with mapi id 14.03.0123.003; Mon, 25 Nov 2013 16:48:55 +0000 From: "Tedeschi, Walfred" To: Pedro Alves CC: "mark.kettenis@xs4all.nl" , "gdb-patches@sourceware.org" Subject: RE: [PATCH v2 1/1] Fix PR16193 - gdbserver aborts. Date: Mon, 25 Nov 2013 17:08:00 -0000 Message-ID: References: <1385394941-26593-1-git-send-email-walfred.tedeschi@intel.com> <52937A8C.9080301@redhat.com> In-Reply-To: <52937A8C.9080301@redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg00789.txt.bz2 Hi Pedro, In fact I didn't like this macro from the beginning on. And liked less when= I saw it with AVX512.=20 Therefore I took the time to rewrite it. It was too awful to read in the wa= y it was. :) :) I will fix the rationale. Thanks and regards, -Fred -----Original Message----- From: Pedro Alves [mailto:palves@redhat.com]=20 Sent: Monday, November 25, 2013 5:28 PM To: Tedeschi, Walfred Cc: mark.kettenis@xs4all.nl; gdb-patches@sourceware.org Subject: Re: [PATCH v2 1/1] Fix PR16193 - gdbserver aborts. On 11/25/2013 03:55 PM, Walfred Tedeschi wrote: > The MPX patch has broken the I386_XSTATE_SIZE macro. For any xcr0=20 > value entered return is 576. This patch fixes that and improves=20 > readability of the macros. Since you still didn't explain it, I took another closer look. I can't believe how long it took me to spot it. I kept staring at the BNDR= EGS and BNDCFG bits, but the issue is really in the AVX check. > /* Get I386 XSAVE extended state size. */ #define=20 > I386_XSTATE_SIZE(XCR0) \ > - (((XCR0) & I386_XSTATE_BNDCFG) !=3D 0 ? I386_XSTATE_BNDCFG_SIZE \ > - : (((XCR0) & I386_XSTATE_BNDREGS) !=3D 0 ? I386_XSTATE_BNDCFG_SIZ= E \ > - : (((XCR0) & I386_XSTATE_AVX_SIZE) !=3D 0 ? I386_XSTATE_AVX_SIZE \ The issue is here: : (((XCR0) & I386_XSTATE_AVX_SIZE) !=3D 0 ? I386_XSTATE_AVX_SIZE \ That should have been: : (((XCR0) & I386_XSTATE_AVX) !=3D 0 ? I386_XSTATE_AVX_SIZE \ Please fix the rationale in the commit log. It's not true that I386_XSTATE= _SIZE _always_ returns 576. For MPX machines, the existing code returns th= e right value. It's only non-MPX AVX machines that get the wrong value. I= OW, this one lines would be the equivalent smallest fix: /* Get I386 XSAVE extended state size. */ #define I386_XSTATE_SIZE(XCR0) \ (((XCR0) & I386_XSTATE_BNDCFG) !=3D 0 ? I386_XSTATE_BNDCFG_SIZE \ : (((XCR0) & I386_XSTATE_BNDREGS) !=3D 0 ? I386_XSTATE_BNDCFG_SIZE \ - : (((XCR0) & I386_XSTATE_AVX_SIZE) !=3D 0 ? I386_XSTATE_AVX_SIZE \ + : (((XCR0) & I386_XSTATE_AVX) !=3D 0 ? I386_XSTATE_AVX_SIZE \ : I386_XSTATE_SSE_SIZE))) Something like this: " The MPX patch has broken the I386_XSTATE_SIZE macro. For AVX machines, it = ends up returning I386_XSTATE_SSE_SIZE. Where it first reads I386_XSTATE_A= VX_SIZE, it should have read I386_XSTATE_AVX: #define I386_XSTATE_SIZE(XCR0) \ (((XCR0) & I386_XSTATE_BNDCFG) !=3D 0 ? I386_XSTATE_BNDCFG_SIZE \ : (((XCR0) & I386_XSTATE_BNDREGS) !=3D 0 ? I386_XSTATE_BNDCFG_SIZE \ - : (((XCR0) & I386_XSTATE_AVX_SIZE) !=3D 0 ? I386_XSTATE_AVX_SIZE \ + : (((XCR0) & I386_XSTATE_AVX) !=3D 0 ? I386_XSTATE_AVX_SIZE \ : I386_XSTATE_SSE_SIZE))) The patch goes a step further and improves readability of the macro, by add= ing a couple other auxiliary macros. 2013-11-25 Walfred Tedeschi * i386-xstate.h (I386_XSTATE_MPX): New Macro. (I386_XSTATE_MPX_MASK): Makes use of I386_XSTATE_MPX. (HAS_MPX): New macro. (HAS_AVX): New macro. (I386_XSTATE_SIZE): Uses HAS_MPX and HAS_AVX. " > 2013-12-25 Walfred Tedeschi Longing for Christmas? ;-) -> Oh! Yah! :) Intel GmbH Dornacher Strasse 1 85622 Feldkirchen/Muenchen, Deutschland Sitz der Gesellschaft: Feldkirchen bei Muenchen Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk Registergericht: Muenchen HRB 47456 Ust.-IdNr./VAT Registration No.: DE129385895 Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052