Re: [PATCH] [RFC] Proposed changes to eliminate 'union mips_instruction' type.

[David Daney <ddaney.cavm@xxxxxxxxx>]

On 01/16/2013 06:16 AM, Ralf Baechle wrote:
> On Tue, Jan 15, 2013 at 02:39:15PM -0800, David Daney wrote:
>
> So this should be fairly readable, far less code and in especially no
> more variants for endianess except a single simple macro.
>
> What do you think?

Very tricky.  I like it.

However, a small change is needed...

[...]
> +#define BITFIELD_FIELD(field, more)					\
> +	field;								\
> +	more
>
>   #elif defined(__MIPSEL__)
[...]
> +#define BITFIELD_FIELD(field, more)					\
> +	more								\
> +	field;
>
>   #else /* !defined (__MIPSEB__) && !defined (__MIPSEL__) */
>   #error "MIPS but neither __MIPSEL__ nor __MIPSEB__?"
>   #endif
>
> +struct j_format {
> +	BITFIELD_FIELD(unsigned int opcode : 6,	/* Jump format */
> +	BITFIELD_FIELD(unsigned int target : 26,

... In the very last BITFIELD_FIELD(), you need a valid token as the 
second parameter, otherwise (according to Pinski) C90 behavior is undefined.

Use a ';'



> +	))
> +};
> +
> +struct i_format {			/* signed immediate format */
> +	BITFIELD_FIELD(unsigned int opcode : 6,
> +	BITFIELD_FIELD(unsigned int rs : 5,
> +	BITFIELD_FIELD(unsigned int rt : 5,
> +	BITFIELD_FIELD(signed int simmediate : 16,
> +	))))
> +};
> +
> +struct u_format {			/* unsigned immediate format */
> +	BITFIELD_FIELD(unsigned int opcode : 6,
> +	BITFIELD_FIELD(unsigned int rs : 5,
> +	BITFIELD_FIELD(unsigned int rt : 5,
> +	BITFIELD_FIELD(unsigned int uimmediate : 16,
> +	))))
> +};
> +
> +struct c_format {			/* Cache (>= R6000) format */
> +	BITFIELD_FIELD(unsigned int opcode : 6,
> +	BITFIELD_FIELD(unsigned int rs : 5,
> +	BITFIELD_FIELD(unsigned int c_op : 3,
> +	BITFIELD_FIELD(unsigned int cache : 2,
> +	BITFIELD_FIELD(unsigned int simmediate : 16,
> +	)))))
> +};
> +
> +struct r_format {			/* Register format */
> +	BITFIELD_FIELD(unsigned int opcode : 6,
> +	BITFIELD_FIELD(unsigned int rs : 5,
> +	BITFIELD_FIELD(unsigned int rt : 5,
> +	BITFIELD_FIELD(unsigned int rd : 5,
> +	BITFIELD_FIELD(unsigned int re : 5,
> +	BITFIELD_FIELD(unsigned int func : 6,
> +	))))))
> +};
> +
> +struct p_format {		/* Performance counter format (R10000) */
> +	BITFIELD_FIELD(unsigned int opcode : 6,
> +	BITFIELD_FIELD(unsigned int rs : 5,
> +	BITFIELD_FIELD(unsigned int rt : 5,
> +	BITFIELD_FIELD(unsigned int rd : 5,
> +	BITFIELD_FIELD(unsigned int re : 5,
> +	BITFIELD_FIELD(unsigned int func : 6,
> +	))))))
> +};BITFIELD_FIELD(
> +
> +struct f_format { 			/* FPU register format */
> +	BITFIELD_FIELD(unsigned int opcode : 6,
> +	BITFIELD_FIELD(unsigned int : 1,
> +	BITFIELD_FIELD(unsigned int fmt : 4,
> +	BITFIELD_FIELD(unsigned int rt : 5,
> +	BITFIELD_FIELD(unsigned int rd : 5,
> +	BITFIELD_FIELD(unsigned int re : 5,
> +	BITFIELD_FIELD(unsigned int func : 6,
> +	)))))))
> +};
> +
> +struct ma_format {		/* FPU multiply and add format (MIPS IV) */
> +	BITFIELD_FIELD(unsigned int opcode : 6,
> +	BITFIELD_FIELD(unsigned int fr : 5,
> +	BITFIELD_FIELD(unsigned int ft : 5,
> +	BITFIELD_FIELD(unsigned int fs : 5,
> +	BITFIELD_FIELD(unsigned int fd : 5,
> +	BITFIELD_FIELD(unsigned int func : 4,
> +	BITFIELD_FIELD(unsigned int fmt : 2,
> +	)))))))
> +};
> +
> +struct b_format {			/* BREAK and SYSCALL */
> +	BITFIELD_FIELD(unsigned int opcode : 6,
> +	BITFIELD_FIELD(unsigned int code : 20,
> +	BITFIELD_FIELD(unsigned int func : 6,
> +	)))
> +};
> +
>   union mips_instruction {
>   	unsigned int word;
>   	unsigned short halfword[2];
> @@ -353,6 +299,7 @@ union mips_instruction {
>   	struct u_format u_format;
>   	struct c_format c_format;
>   	struct r_format r_format;
> +	struct p_format p_format;
>   	struct f_format f_format;
>   	struct ma_format ma_format;
>   	struct b_format b_format;