This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] New bitflags type and eflags on i386/x86-64
- From: Daniel Jacobowitz <drow at mvista dot com>
- To: Michal Ludvig <mludvig at suse dot cz>
- Cc: Andrew Cagney <ac131313 at redhat dot com>, gdb-patches at sources dot redhat dot com
- Date: Thu, 29 Aug 2002 10:21:20 -0400
- Subject: Re: [RFA] New bitflags type and eflags on i386/x86-64
- References: <3CC42DA0.9070906@suse.cz> <3D6BF1D5.70409@ges.redhat.com> <3D6E231D.8060906@suse.cz>
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