Re: [rfc] Replace x86 register macros

["Ulrich Weigand" <uweigand@de.ibm.com>]

Markus Deuling wrote:

> in some places used by x86/amd64 architecture there is a weird mechanism to 
> get at the number of registers or the registers themselves which make the code hard to
> read.
> 
> One example is i386_sse_regnum_p in i386-tdep.c where two macros I387_ST0_REGNUM and
> I387_NUM_XMM_REGS have to be defined so that other two macros I387_XMM0_REGNUM and
> I387_MXCSR_REGNUM are valid. 

This was intended as a way to support both i386 and amd64 with
the same code, even though register numbers differ.

See the comment in i387-tdep.h:

/* Because the number of general-purpose registers is different for
   AMD64, the floating-point registers and SSE registers get shifted.
   The following definitions are intended to help writing code that
   needs the register numbers of floating-point registers and SSE
   registers.  In order to use these, one should provide a definition
   for I387_ST0_REGNUM, and possibly I387_NUM_XMM_REGS, preferably by
   using a local "#define" in the body of the function that uses this.
   Please "#undef" them before the end of the function.  */

#define I387_FCTRL_REGNUM       (I387_ST0_REGNUM + 8)
#define I387_FSTAT_REGNUM       (I387_FCTRL_REGNUM + 1)
#define I387_FTAG_REGNUM        (I387_FCTRL_REGNUM + 2)
#define I387_FISEG_REGNUM       (I387_FCTRL_REGNUM + 3)
#define I387_FIOFF_REGNUM       (I387_FCTRL_REGNUM + 4)
#define I387_FOSEG_REGNUM       (I387_FCTRL_REGNUM + 5)
#define I387_FOOFF_REGNUM       (I387_FCTRL_REGNUM + 6)
#define I387_FOP_REGNUM         (I387_FCTRL_REGNUM + 7)
#define I387_XMM0_REGNUM        (I387_ST0_REGNUM + 16)
#define I387_MXCSR_REGNUM       (I387_XMM0_REGNUM + I387_NUM_XMM_REGS)


I agree that it would be nicer to handle this in a different
fashion, but I don't like this approach either:

> -  return (I387_XMM0_REGNUM <= regnum && regnum < I387_MXCSR_REGNUM);

> +  /* True if REGNUM in [st0_regnum + 16, st0_regnum + 16 + num_xmm_regs).  */
> +  return (regnum >= tdep->st0_regnum + 16
> +	  && regnum < tdep->st0_regnum + 16 + tdep->num_xmm_regs);

This leads to hard-coding those magic numbers like 16 all
over the place.  Having a symbolic name for these is much better.

I'd suggest to keep the I387_..._REGNUM macros, but add a tdep
parameter to them.  All users would need to be changed to pass
in the proper tdep, but that only makes the existing dependency
explicit.

#define I387_FCTRL_REGNUM(tdep)       ((tdep)->st0_regnum + 8)
#define I387_FSTAT_REGNUM(tdep)       (I387_FCTRL_REGNUM (tdep) + 1)
...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com