From: "H.J. Lu" <hjl.tools@gmail.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [PATCH 3/3] x86: clean up after removal of support for gcc <= 2.8.1
Date: Tue, 29 Nov 2022 16:00:54 -0800 [thread overview]
Message-ID: <CAMe9rOp=NC3YsG26bR+TF5r9tgVpBdgDQ3qvRxd+M5Y8P_mZ4Q@mail.gmail.com> (raw)
In-Reply-To: <5dd8b2df-62f2-a551-4b35-f3df66d57e04@suse.com>
On Thu, Nov 24, 2022 at 12:58 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> At the very least a comment in process_operands() is stale. Beyond that
> there are effectively two options:
> 1) It is possible that FADDP and FMULP were mistakenly not marked as
> being in need of dealing with the compiler anomaly, and hence the
> respective templates weren't removed at the time when they should
> have been.
> 2) It is also possible that there are indeed uses known beyond compiler
> generated output for these two commutative opcodes, and hence the
> templates need to stay.
> To be on the safe side assume 2: Update the comment and fold the
> templates into their "normal" ones (utilizing D), adjusting consuming
> code accordingly.
>
> For FMULP also add a comment paralleling a similar one FADDP has.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -6806,7 +6806,8 @@ match_template (char mnem_suffix)
> found_reverse_match = 0;
> else if (operand_types[0].bitfield.tbyte)
> {
> - found_reverse_match = Opcode_FloatD;
> + if (t->opcode_modifier.operandconstraint != UGH)
> + 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))
> @@ -7997,29 +7998,31 @@ process_operands (void)
> {
> /* The register or float register operand is in operand
> 0 or 1. */
> - unsigned int op = i.tm.operand_types[0].bitfield.class != Reg;
> + const reg_entry *r = i.op[0].regs;
>
> + if (i.imm_operands
> + || (r->reg_type.bitfield.instance == Accum && i.op[1].regs))
> + r = i.op[1].regs;
> /* Register goes in low 3 bits of opcode. */
> - i.tm.base_opcode |= i.op[op].regs->reg_num;
> - if ((i.op[op].regs->reg_flags & RegRex) != 0)
> + i.tm.base_opcode |= r->reg_num;
> + if ((r->reg_flags & RegRex) != 0)
> i.rex |= REX_B;
> if (!quiet_warnings && i.tm.opcode_modifier.operandconstraint == UGH)
> {
> - /* Warn about some common errors, but press on regardless.
> - The first case can be generated by gcc (<= 2.8.1). */
> - if (i.operands == 2)
> - {
> - /* Reversed arguments on faddp, fsubp, etc. */
> - as_warn (_("translating to `%s %s%s,%s%s'"), i.tm.name,
> - register_prefix, i.op[!intel_syntax].regs->reg_name,
> - register_prefix, i.op[intel_syntax].regs->reg_name);
> - }
> - else
> + /* Warn about some common errors, but press on regardless. */
> + if (i.operands != 2)
> {
> /* Extraneous `l' suffix on fp insn. */
> as_warn (_("translating to `%s %s%s'"), i.tm.name,
> register_prefix, i.op[0].regs->reg_name);
> }
> + else if (i.op[0].regs->reg_type.bitfield.instance != Accum)
> + {
> + /* Reversed arguments on faddp or fmulp. */
> + as_warn (_("translating to `%s %s%s,%s%s'"), i.tm.name,
> + register_prefix, i.op[!intel_syntax].regs->reg_name,
> + register_prefix, i.op[intel_syntax].regs->reg_name);
> + }
> }
> }
>
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -686,11 +686,10 @@ fadd, 0xdec1, None, CpuFP, NoSuf|Ugh|ATT
> fadd, 0xd8, 0, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Dword|Qword|Unspecified|BaseIndex }
> fiadd, 0xde, 0, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Word|Dword|Unspecified|BaseIndex }
>
> -faddp, 0xdec0, None, CpuFP, NoSuf, { FloatAcc, FloatReg }
> +faddp, 0xdec0, None, CpuFP, D|NoSuf|Ugh, { FloatAcc, FloatReg }
> faddp, 0xdec0, None, CpuFP, NoSuf, { FloatReg }
> // alias for faddp %st, %st(1)
> faddp, 0xdec1, None, CpuFP, NoSuf, {}
> -faddp, 0xdec0, None, CpuFP, NoSuf|Ugh, { FloatReg, FloatAcc }
>
> // subtract
> fsub, 0xd8e0, None, CpuFP, NoSuf, { FloatReg }
> @@ -732,10 +731,10 @@ fmul, 0xdec9, None, CpuFP, NoSuf|Ugh|ATT
> fmul, 0xd8, 1, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Dword|Qword|Unspecified|BaseIndex }
> fimul, 0xde, 1, CpuFP, Modrm|FloatMF|No_bSuf|No_wSuf|No_qSuf|No_ldSuf, { Word|Dword|Unspecified|BaseIndex }
>
> -fmulp, 0xdec8, None, CpuFP, NoSuf, { FloatAcc, FloatReg }
> +fmulp, 0xdec8, None, CpuFP, D|NoSuf|Ugh, { FloatAcc, FloatReg }
> fmulp, 0xdec8, None, CpuFP, NoSuf, { FloatReg }
> +// alias for fmulp %st, %st(1)
> fmulp, 0xdec9, None, CpuFP, NoSuf, {}
> -fmulp, 0xdec8, None, CpuFP, NoSuf|Ugh, { FloatReg, FloatAcc }
>
> // divide
> fdiv, 0xd8f0, None, CpuFP, NoSuf, { FloatReg }
>
OK.
Thanks.
--
H.J.
prev parent reply other threads:[~2022-11-30 0:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-24 8:56 [PATCH 0/3] x86: FPU insn handling simplifications Jan Beulich
2022-11-24 8:57 ` [PATCH 1/3] x86: extend FPU test coverage for AT&T / Intel mnemonic differences Jan Beulich
2022-11-29 23:59 ` H.J. Lu
2022-11-24 8:57 ` [PATCH 2/3] x86: drop FloatR Jan Beulich
2022-11-30 0:00 ` H.J. Lu
2022-11-24 8:58 ` [PATCH 3/3] x86: clean up after removal of support for gcc <= 2.8.1 Jan Beulich
2022-11-28 23:21 ` H.J. Lu
2022-11-29 9:02 ` Jan Beulich
2022-11-30 0:00 ` H.J. Lu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMe9rOp=NC3YsG26bR+TF5r9tgVpBdgDQ3qvRxd+M5Y8P_mZ4Q@mail.gmail.com' \
--to=hjl.tools@gmail.com \
--cc=binutils@sourceware.org \
--cc=jbeulich@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).