RE: [PATCH v2 2/2] gdb, gdbserver, python, testsuite: Remove MPX.

["Schimpe, Christina" <christina.schimpe@intel.com>]

> > diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c index
> > 675ee8d2e81..634a9a7ab85 100644
> > --- a/gdb/i387-tdep.c
> > +++ b/gdb/i387-tdep.c
> > @@ -818,30 +818,6 @@ static int xsave_xmm_avx512_offset[] =
> >    (xsave + (tdep)->xsave_layout.zmm_offset				\
> >     + xsave_xmm_avx512_offset[regnum - I387_XMM16_REGNUM (tdep)])
> >
> > -/* At xsave_bndregs_offset[REGNUM] you'll find the relative offset
> > -   within the BNDREGS region of the XSAVE extended state where the GDB
> > -   register BND0R + REGNUM is stored.  */
> > -
> > -static int xsave_bndregs_offset[] = {
> > -  0 * 16,			/* bnd0r...bnd3r registers.  */
> > -  1 * 16,
> > -  2 * 16,
> > -  3 * 16
> > -};
> > -
> > -#define XSAVE_BNDREGS_ADDR(tdep, xsave, regnum)
> > 	\
> > -  (xsave + (tdep)->xsave_layout.bndregs_offset
> 	\
> > -   + xsave_bndregs_offset[regnum - I387_BND0R_REGNUM (tdep)])
> > -
> > -static int xsave_bndcfg_offset[] = {
> > -  0 * 8,			/* bndcfg ... bndstatus.  */
> > -  1 * 8,
> > -};
> > -
> > -#define XSAVE_BNDCFG_ADDR(tdep, xsave, regnum)			\
> > -  (xsave + (tdep)->xsave_layout.bndcfg_offset			\
> > -   + xsave_bndcfg_offset[regnum - I387_BNDCFGU_REGNUM (tdep)])
> > -
> >  /* At xsave_avx512_k_offset[REGNUM] you'll find the relative offset
> >     within the K region of the XSAVE extended state where the AVX512
> >     opmask register K0 + REGNUM is stored.  */ @@ -944,8 +920,8 @@
> > i387_guess_xsave_layout (uint64_t xcr0, size_t xsave_size,
> >      {
> >        /* Intel CPUs supporting PKRU.  */
> >        layout.avx_offset = 576;
> > -      layout.bndregs_offset = 960;
> > -      layout.bndcfg_offset = 1024;
> > +      /* APX will take up the space left behind by the deprecated MPX
> > +	 registers.  */
> >        layout.k_offset = 1088;
> >        layout.zmm_h_offset = 1152;
> >        layout.zmm_offset = 1664;
> > @@ -964,19 +940,12 @@ i387_guess_xsave_layout (uint64_t xcr0, size_t
> > xsave_size,
> >      {
> >        /* Intel CPUs supporting AVX512.  */
> >        layout.avx_offset = 576;
> > -      layout.bndregs_offset = 960;
> > -      layout.bndcfg_offset = 1024;
> > +      /* APX will take up the space left behind by the deprecated MPX
> > +	 registers.  */
> >        layout.k_offset = 1088;
> >        layout.zmm_h_offset = 1152;
> >        layout.zmm_offset = 1664;
> >      }
> > -  else if (HAS_MPX (xcr0) && xsave_size == 1088)
> > -    {
> > -      /* Intel CPUs supporting MPX.  */
> > -      layout.avx_offset = 576;
> > -      layout.bndregs_offset = 960;
> > -      layout.bndcfg_offset = 1024;
> > -    }
> 
> Mhm, I wonder if that doesn't unnecessarily make reading corefiles from old
> CPUs worse than before. If the CPU has MPX as the highest supported
> feature and xsave_size is 1088, we now will return 0, and will therefore create
> some generic target description with the SSE mask, and therefore we will no
> longer be able to read the AVX registers (If I followed the code correctly).
> I wonder if keeping HAS_MPX and this if statement (minus the bnd
> stuff) here wouldn't be better. That way, we at least keep AVX support in
> corefiles for those older CPUs.

I agree there could be problems for HW supporting MPX as highest supported feature.
I wonder if it's not enough just to add "|| (xsave_size == 1088)" to the latest else if: 

~~~
  else if (HAS_AVX (xcr0) && (xsave_size == 832 || xsave_size == 1088))
     {
      /* Intel and AMD CPUs supporting AVX.  */
      layout.avx_offset = 576;
    }
~~~

But I'd also be ok with keeping HAS_MPX.

Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928