Re: [RFA] New bitflags type and eflags on i386/x86-64

[Daniel Jacobowitz <drow@mvista.com>]

On Thu, Aug 29, 2002 at 03:35:25PM +0200, Michal Ludvig wrote:
> Andrew Cagney wrote:
> >However, given that there are two implementations, I get the feeling 
> >that [possibly fee paying] users want something.
> 
> Here is the reworked patch. The print function was moved to valprint.c 
> and is called from all relevant <language>-valprint.c modules.
> Also I've added support for MXCSR register both for i386 and x86-64.
> 
> Comments? Can I commit it?

Well, actually, I like it.  Some textual changes and comments:

> +/*
> + * - The following three functions are intended to be used for BitFlags 
> + *   types (ie i386's EFLAGS register).

e.g., not ie.

> + * - As a BitFlag we understand an integer where some bits may have 
> + *   a symbolic names that would be printed when the bit is set.

"A BitFlags type is an integer where bits may have a symbolic name to
be printed when the bit is set."

> + * - Printing is done in c_val_print() under a TYPE_CODE_FLAGS label.

<lang>_val_print perhaps?

> + * - Add a symbolic name of relevant bits using add_flag_name() after 
> + *   an initialisation of your type.
> + */

"Add symbolic names for relevant bits using add_flag_name() after
initializing the BitFlags type."

> +struct type *
> +init_flags_type (int bitlength, char *name, struct objfile *objfile)
> +{
> +  register struct type *type;
> +  
> +  type = alloc_type (objfile);
> +  
> +  TYPE_CODE (type) = TYPE_CODE_FLAGS;
> +  TYPE_LENGTH (type) = bitlength / 8 + ( bitlength % 8 ? 1 : 0 );

Don't hardcode a magic eight.  How about:
  (bitlength + TARGET_CHAR_BIT - 1) / TARGET_CHAR_BIT
(I think it's called TARGET_CHAR_BIT at least)

> @@ -2555,6 +2614,8 @@ rank_one_type (struct type *parm, struct
>  	    return INTEGER_PROMOTION_BADNESS;
>  	  else
>  	    return INTEGER_COERCION_BADNESS;
> +        case TYPE_CODE_FLAGS:
> +	  return 0;
>  	case TYPE_CODE_ENUM:
>  	case TYPE_CODE_CHAR:
>  	case TYPE_CODE_RANGE:

No, INTEGER_PROMOTION_BADNESS.  Just add the case, not the return.

> @@ -3433,6 +3497,49 @@ build_gdbtypes (void)
>      init_type (TYPE_CODE_INT, TARGET_BFD_VMA_BIT / 8,
>  	       TYPE_FLAG_UNSIGNED,
>  	       "__bfd_vma", (struct objfile *) NULL);
> +
> +  builtin_type_i386_eflags = 
> +    init_flags_type (32 /* EFLAGS_LENGTH */, 
> +    	       "__i386_eflags", (struct objfile *) NULL);
> +  add_flag_name (builtin_type_i386_eflags, 0, "CF");
> +  add_flag_ignore (builtin_type_i386_eflags, 1);
> +  add_flag_name (builtin_type_i386_eflags, 2, "PF");
> +  add_flag_name (builtin_type_i386_eflags, 4, "AF");
> +  add_flag_name (builtin_type_i386_eflags, 6, "ZF");
> +  add_flag_name (builtin_type_i386_eflags, 7, "SF");
> +  add_flag_name (builtin_type_i386_eflags, 8, "TF");
> +  add_flag_name (builtin_type_i386_eflags, 9, "IF");
> +  add_flag_name (builtin_type_i386_eflags, 10, "DF");
> +  add_flag_name (builtin_type_i386_eflags, 11, "OF");
> +  add_flag_ignore (builtin_type_i386_eflags, 12);
> +  add_flag_ignore (builtin_type_i386_eflags, 13);
> +  add_flag_name (builtin_type_i386_eflags, 14, "NT");
> +  add_flag_name (builtin_type_i386_eflags, 16, "RF");
> +  add_flag_name (builtin_type_i386_eflags, 17, "VM");
> +  add_flag_name (builtin_type_i386_eflags, 18, "AC");
> +  add_flag_name (builtin_type_i386_eflags, 19, "VIF");
> +  add_flag_name (builtin_type_i386_eflags, 20, "VIP");
> +  add_flag_name (builtin_type_i386_eflags, 21, "ID");
> +  
> +  builtin_type_simd_mxcsr = 
> +    init_flags_type (32 /* EFLAGS_LENGTH */, 
> +    	       "__simd_mxcsr", (struct objfile *) NULL);
> +  add_flag_name (builtin_type_simd_mxcsr, 0, "IE");
> +  add_flag_name (builtin_type_simd_mxcsr, 1, "DE");
> +  add_flag_name (builtin_type_simd_mxcsr, 2, "ZE");
> +  add_flag_name (builtin_type_simd_mxcsr, 3, "OE");
> +  add_flag_name (builtin_type_simd_mxcsr, 4, "UE");
> +  add_flag_name (builtin_type_simd_mxcsr, 5, "PE");
> +  add_flag_name (builtin_type_simd_mxcsr, 6, "DAZ");
> +  add_flag_name (builtin_type_simd_mxcsr, 7, "IM");
> +  add_flag_name (builtin_type_simd_mxcsr, 8, "DM");
> +  add_flag_name (builtin_type_simd_mxcsr, 9, "ZM");
> +  add_flag_name (builtin_type_simd_mxcsr, 10, "OM");
> +  add_flag_name (builtin_type_simd_mxcsr, 11, "UM");
> +  add_flag_name (builtin_type_simd_mxcsr, 12, "PM");
> +  add_flag_name (builtin_type_simd_mxcsr, 13, "RC1");
> +  add_flag_name (builtin_type_simd_mxcsr, 14, "RC2");
> +  add_flag_name (builtin_type_simd_mxcsr, 15, "FZ");
>  }
>  
>  

Do these really need to be in common code?  That's gross.  Yes, I know
a whole lot of others are, but those are all floatformats or standard
vectors.

This should be in i386-tdep.c.

> @@ -926,6 +927,10 @@ extern struct type *builtin_type_void_fu
>  
>  /* The target CPU's address type.  This is the ISA address size. */
>  extern struct type *builtin_type_CORE_ADDR;
> +
> +/* Type for i386 EFLAGS register.  */
> +extern struct type *builtin_type_i386_eflags;
> +
>  /* The symbol table address type.  Some object file formats have a 32
>     bit address type even though the TARGET has a 64 bit pointer type
>     (cf MIPS). */
> @@ -951,6 +956,9 @@ extern struct type *builtin_type_v8qi;
>  extern struct type *builtin_type_v8hi;
>  extern struct type *builtin_type_v4hi;
>  extern struct type *builtin_type_v2si;
> +
> +/* MXCSR SIMD control register.  */
> +extern struct type *builtin_type_simd_mxcsr;
>  
>  /* Type for 64 bit vectors. */
>  extern struct type *builtin_type_vec64;

Ditto.


The rest of it looks good to me.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer