Re: [PATCH 2/3] x86: drop FloatR

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Nov 24, 2022 at 12:57 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> There are just 4 templates using it, which can be easily identified by
> other means, as D is set only on a very limited number of FPU templates.
> Also move the respective conditional out of the code path taken by all
> "reverse match" insns (it probably should have been this way already
> before, to avoid the one conditional in the common case).
>
> With this the templates which had FloatR dropped no longer differ from
> their AT&T syntax + mnemonic counterparts - the only difference is now
> which of the two would be recognized. For this, however, we don't need
> two templates - we can simply arrange the condition for setting
> Opcode_FloatR accordingly.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -6800,12 +6800,18 @@ match_template (char mnem_suffix)
>                   specific_error = progress (i.error);
>                   continue;
>                 }
> -             /* found_reverse_match holds which of D or FloatR
> +             /* found_reverse_match holds which variant of D
>                  we've found.  */
>               if (!t->opcode_modifier.d)
>                 found_reverse_match = 0;
>               else if (operand_types[0].bitfield.tbyte)
> -               found_reverse_match = Opcode_FloatD;
> +               {
> +                 found_reverse_match = Opcode_FloatD;
> +                 /* FSUB{,R} and FDIV{,R} may need a 2nd bit flipped.  */
> +                 if ((t->base_opcode & 0x20)
> +                     && (intel_syntax || intel_mnemonic))
> +                   found_reverse_match |= Opcode_FloatR;
> +               }
>               else if (t->opcode_modifier.vexsources)
>                 {
>                   found_reverse_match = Opcode_VexW;
> @@ -6820,8 +6826,6 @@ match_template (char mnem_suffix)
>                                       ? Opcode_ExtD : Opcode_SIMD_IntD;
>               else
>                 found_reverse_match = Opcode_D;
> -             if (t->opcode_modifier.floatr)
> -               found_reverse_match |= Opcode_FloatR;
>             }
>           else
>             {
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -731,7 +731,6 @@ static bitfield opcode_modifiers[] =
>    BITFIELD (Modrm),
>    BITFIELD (Jump),
>    BITFIELD (FloatMF),
> -  BITFIELD (FloatR),
>    BITFIELD (Size),
>    BITFIELD (CheckRegSize),
>    BITFIELD (OperandConstraint),
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -487,8 +487,6 @@ enum
>    Jump,
>    /* FP insn memory format bit, sized by 0x4 */
>    FloatMF,
> -  /* src/dest swap for floats. */
> -  FloatR,
>    /* needs size prefix if in 32-bit mode */
>  #define SIZE16 1
>    /* needs size prefix if in 16-bit mode */
> @@ -743,7 +741,6 @@ typedef struct i386_opcode_modifier
>    unsigned int modrm:1;
>    unsigned int jump:3;
>    unsigned int floatmf:1;
> -  unsigned int floatr:1;
>    unsigned int size:2;
>    unsigned int checkregsize:1;
>    unsigned int operandconstraint:4;
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -694,11 +694,10 @@ faddp, 0xdec0, None, CpuFP, NoSuf|Ugh, {
>
>  // subtract
>  fsub, 0xd8e0, None, CpuFP, NoSuf, { FloatReg }
> -fsub, 0xd8e0, None, CpuFP, D|NoSuf|ATTMnemonic|ATTSyntax, { FloatReg, FloatAcc }
> +fsub, 0xd8e0, None, CpuFP, D|NoSuf, { FloatReg, FloatAcc }
>  // alias for fsubp
>  fsub, 0xdee1, None, CpuFP, NoSuf|Ugh|ATTMnemonic|ATTSyntax, {}
>  fsub, 0xdee9, None, CpuFP, NoSuf|Ugh|ATTMnemonic, {}
> -fsub, 0xd8e0, None, CpuFP, NoSuf|D|FloatR, { FloatReg, FloatAcc }
>  fsub, 0xd8, 4, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Dword|Qword|Unspecified|BaseIndex }
>  fisub, 0xde, 4, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Word|Dword|Unspecified|BaseIndex }
>
> @@ -711,11 +710,10 @@ fsubp, 0xdee9, None, CpuFP, NoSuf, {}
>
>  // subtract reverse
>  fsubr, 0xd8e8, None, CpuFP, NoSuf, { FloatReg }
> -fsubr, 0xd8e8, None, CpuFP, D|NoSuf|ATTMnemonic|ATTSyntax, { FloatReg, FloatAcc }
> +fsubr, 0xd8e8, None, CpuFP, D|NoSuf, { FloatReg, FloatAcc }
>  // alias for fsubrp
>  fsubr, 0xdee9, None, CpuFP, NoSuf|Ugh|ATTMnemonic|ATTSyntax, {}
>  fsubr, 0xdee1, None, CpuFP, NoSuf|Ugh|ATTMnemonic, {}
> -fsubr, 0xd8e8, None, CpuFP, NoSuf|D|FloatR, { FloatReg, FloatAcc }
>  fsubr, 0xd8, 5, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Dword|Qword|Unspecified|BaseIndex }
>  fisubr, 0xde, 5, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Word|Dword|Unspecified|BaseIndex }
>
> @@ -741,11 +739,10 @@ fmulp, 0xdec8, None, CpuFP, NoSuf|Ugh, {
>
>  // divide
>  fdiv, 0xd8f0, None, CpuFP, NoSuf, { FloatReg }
> -fdiv, 0xd8f0, None, CpuFP, D|NoSuf|ATTMnemonic|ATTSyntax, { FloatReg, FloatAcc }
> +fdiv, 0xd8f0, None, CpuFP, D|NoSuf, { FloatReg, FloatAcc }
>  // alias for fdivp
>  fdiv, 0xdef1, None, CpuFP, NoSuf|Ugh|ATTMnemonic|ATTSyntax, {}
>  fdiv, 0xdef9, None, CpuFP, NoSuf|Ugh|ATTMnemonic, {}
> -fdiv, 0xd8f0, None, CpuFP, NoSuf|D|FloatR, { FloatReg, FloatAcc }
>  fdiv, 0xd8, 6, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Dword|Qword|Unspecified|BaseIndex }
>  fidiv, 0xde, 6, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Word|Dword|Unspecified|BaseIndex }
>
> @@ -758,11 +755,10 @@ fdivp, 0xdef9, None, CpuFP, NoSuf, {}
>
>  // divide reverse
>  fdivr, 0xd8f8, None, CpuFP, NoSuf, { FloatReg }
> -fdivr, 0xd8f8, None, CpuFP, D|NoSuf|ATTMnemonic|ATTSyntax, { FloatReg, FloatAcc }
> +fdivr, 0xd8f8, None, CpuFP, D|NoSuf, { FloatReg, FloatAcc }
>  // alias for fdivrp
>  fdivr, 0xdef9, None, CpuFP, NoSuf|Ugh|ATTMnemonic|ATTSyntax, {}
>  fdivr, 0xdef1, None, CpuFP, NoSuf|Ugh|ATTMnemonic, {}
> -fdivr, 0xd8f8, None, CpuFP, NoSuf|D|FloatR, { FloatReg, FloatAcc }
>  fdivr, 0xd8, 7, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Dword|Qword|Unspecified|BaseIndex }
>  fidivr, 0xde, 7, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Word|Dword|Unspecified|BaseIndex }
>
>

OK.

Thanks.

-- 
H.J.