Re: [PATCH] x86/math-emu: Add support for FCMOVcc and F[U]COMI[P] insns

[Ingo Molnar <mingo@kernel.org>]

* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> On 08/24/2015 08:50 AM, Ingo Molnar wrote:
> > 
> > * Ingo Molnar <mingo@kernel.org> wrote:
> > 
> >>
> >> * Denys Vlasenko <dvlasenk@redhat.com> wrote:
> >>
> >>> I propose the table to be commented like shown below:
> >>>
> >>> /*Opcode: d8      d9        da      db       dc      dd      de      df */
> >>> /*c0..f*/ fadd__, fld_i_,   fcmovb, fcmovnb, fadd_i, ffree_, faddp_, _df_c0_,
> >>> /*c8..f*/ fmul__, fxch_i,   fcmove, fcmovne, fmul_i, _dd_c8_,fmulp_, _df_c8_,
> >>> /*d0..7*/ fcom_st,fp_nop,   fcmovbe,fcmovnbe,_dc_d0_,fst_i_, _de_d0_,_df_d0_,
> >>> /*d8..f*/ fcompst,_d9_d8_,  fcmovu, fcmovnu, _dc_d8_,fstp_i, fcompp, _df_d8_,
> >>> /*e0..7*/ fsub__, FPU_etc,  __BAD__,finit_,  fsubri, fucom_, fsubrp, fstsw_,
> >>> /*e8..f*/ fsubr_, fconst,   fucompp,fucomi_, fsub_i, fucomp, fsubp_, fucomip,
> >>> /*f0..7*/ fdiv__, FPU_triga,__BAD__,fcomi_,  fdivri, __BAD__,fdivrp, fcomip,
> >>> /*f8..f*/ fdivr_, FPU_trigb,__BAD__,__BAD__, fdiv_i, __BAD__,fdivp_, __BAD__,
> >>
> >> I agree with that, but please fix the vertical alignment (like my patch did), as 
> >> the table is pretty hard to navigate in this form.
> > 
> > and by that I mean to split each line into two, to have groups of 4 instructions 
> > and enough tabs between them.
> 
> How would you want it? Like this?
> 
> /*Opcode: d8       d9         da       db */
> /*        dc       dd         de       df */
> /*c0..f*/ fadd__,  fld_i_,    fcmovb,  fcmovnb,
> /*c0..f*/ fadd_i,  ffree_,    faddp_,  _df_c0_,
> /*c8..f*/ fmul__,  fxch_i,    fcmove,  fcmovne,
> /*c8..f*/ fmul_i,  _dd_c8_,   fmulp_,  _df_c8_,
> /*d0..7*/ fcom_st, fp_nop,    fcmovbe, fcmovnbe,
> /*d0..7*/ _dc_d0_, fst_i_,    _de_d0_, _df_d0_,
> /*d8..f*/ fcompst, _d9_d8_,   fcmovu,  fcmovnu,
> /*d8..f*/ _dc_d8_, fstp_i,    fcompp,  _df_d8_,
> /*e0..7*/ fsub__,  FPU_etc,   __BAD__, finit_,
> /*e0..7*/ fsubri,  fucom_,    fsubrp,  fstsw_,
> /*e8..f*/ fsubr_,  fconst,    fucompp, fucomi_,
> /*e8..f*/ fsub_i,  fucomp,    fsubp_,  fucomip,
> /*f0..7*/ fdiv__,  FPU_triga, __BAD__, fcomi_,
> /*f0..7*/ fdivri,  __BAD__,   fdivrp,  fcomip,
> /*f8..f*/ fdivr_,  FPU_trigb, __BAD__, __BAD__,
> /*f8..f*/ fdiv_i,  __BAD__,   fdivp_,  __BAD__,

Yeah, although we could afford a few more tabs and spaces to make it less 
claustrophobic, right?

Something like this:

 /* c0..f */		fadd__,		fld_i_,		fcmovb,		fcmovnb,
 /* c0..f */		fadd_i,		ffree_,		faddp_,		_df_c0_,
 /* c8..f */		fmul__,		fxch_i,		fcmove,		fcmovne,
 /* c8..f */		fmul_i,		_dd_c8_,	fmulp_,		_df_c8_,
 /* d0..7 */		fcom_st,	fp_nop,		fcmovbe,	fcmovnbe,
 /* d0..7 */		_dc_d0_,	fst_i_,		_de_d0_,	_df_d0_,
 /* d8..f */		fcompst,	_d9_d8_		fcmovu,		fcmovnu,
 /* d8..f */		_dc_d8_,	fstp_i,		fcompp,		_df_d8_,
 /* e0..7 */		fsub__,		FPU_etc,	__BAD__,	finit_,
 /* e0..7 */		fsubri,		fucom_,		fsubrp,		fstsw_,
 /* e8..f */		fsubr_,		fconst,		fucompp,	fucomi_,
 /* e8..f */		fsub_i,		fucomp,		fsubp_,		fucomip,
 /* f0..7 */		fdiv__,		FPU_triga,	__BAD__,	fcomi_,
 /* f0..7 */		fdivri,		__BAD__,	fdivrp,		fcomip,
 /* f8..f */		fdivr_,		FPU_trigb,	__BAD__,	__BAD__,
 /* f8..f */		fdiv_i,		__BAD__,	fdivp_,		__BAD__,

looks pretty good to me.

Thanks,

	Ingo