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

Daniel Jacobowitz
2002-08-29T14:20:16Z

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