[PATCH] x86-64: Support APX NF TLS IE with 2 operands

[H.J. Lu]

On Wed, Jul 3, 2024, 3:10 PM Jan Beulich <jbeulich@suse.com> wrote:

> On 03.07.2024 08:48, H.J. Lu wrote:
> > On Wed, Jul 3, 2024, 2:43 PM Jan Beulich <jbeulich@suse.com> wrote:
> >
> >> On 03.07.2024 03:55, H.J. Lu wrote:
> >>> On Wed, Jul 3, 2024, 9:19 AM kong lingling <lingling.kong7@gmail.com>
> >> wrote:
> >>>
> >>>> Support APX NF TLS IE with 2 operands.Verify it with ld and gold.
> >>>>
> >>>> gas/
> >>>>
> >>>>         * config/tc-i386.c (md_assemble): Allow APX NF TLS IE with
> >>>>         2 operands.
> >>>>         * testsuite/gas/i386/x86-64-gottpoff.d: Updated.
> >>>>         * testsuite/gas/i386/x86-64-gottpoff.s: Add APX NF TLS IE
> >>>>         tests with 2 operands.
> >>>>
> >>>> gold/
> >>>>
> >>>>         * testsuite/x86_64_ie_to_le.s: Add APX NF TLS IE tests with
> >>>>         2 operands.
> >>>>         * testsuite/x86_64_ie_to_le.sh: Updated.
> >>>>
> >>>> ld/
> >>>>
> >>>>         * testsuite/ld-x86-64/tlsbindesc.s: Add APX NF TLS IE tests
> >>>>         with 2 operands.
> >>>>         * testsuite/ld-x86-64/tlsbindesc.d: Updated.
> >>>>         * testsuite/ld-x86-64/tlsbindesc.rd: Likewise.
> >>>> ---
> >>>>  gas/config/tc-i386.c                     | 10 +++++--
> >>>>  gas/testsuite/gas/i386/x86-64-gottpoff.d |  4 +++
> >>>>  gas/testsuite/gas/i386/x86-64-gottpoff.s | 10 +++++++
> >>>>  gold/testsuite/x86_64_ie_to_le.s         |  1 +
> >>>>  gold/testsuite/x86_64_ie_to_le.sh        |  1 +
> >>>>  ld/testsuite/ld-x86-64/tlsbindesc.dd     | 12 ++++++++
> >>>>  ld/testsuite/ld-x86-64/tlsbindesc.rd     | 36
> ++++++++++++------------
> >>>>  ld/testsuite/ld-x86-64/tlsbindesc.s      |  4 +++
> >>>>  8 files changed, 58 insertions(+), 20 deletions(-)
> >>>>
> >>>> --- a/gas/config/tc-i386.c
> >>>> +++ b/gas/config/tc-i386.c
> >>>> @@ -7545,8 +7545,14 @@ md_assemble (char *line)
> >>>>                 && i.base_reg
> >>>>                 && i.base_reg->reg_num == RegIP
> >>>>                 && i.tm.operand_types[0].bitfield.class == Reg
> >>>> -               && i.tm.operand_types[2].bitfield.class == Reg)
> >>>> -             /* Allow APX: add %reg1, foo@gottpoff(%rip), %reg2.  */
> >>>> +               && (i.tm.operand_types[2].bitfield.class == Reg
> >>>> +                   || i.tm.operands == 2))
> >>>> +             /* Allow APX:
> >>>> +                add %reg1, foo@gottpoff(%rip), %reg2
> >>>> +                add foo@gottpoff(%rip), %reg, %reg2
> >>>> +                {nf} add foo@gottpoff(%rip), %reg
> >>>> +                {nf} add %reg1, foo@gottpoff(%rip), %reg2
> >>>> +                {nf} add foo@gottpoff(%rip), %reg, %reg2.  */
> >>>>               break;
> >>>>             /* Fall through.  */
> >>>>           case BFD_RELOC_386_TLS_GOTIE:
> >>>> [...]
> >>>> --
> >>>> 2.31.1
> >>>>
> >>>
> >>> OK.
> >>
> >> H.J., please don't do this when you're well aware that earlier comments
> >> of others (me in this case) were not addressed. The code left in context
> >> above _STILL_ permits memory destinations for the 2-operand case,
> despite
> >> the comment saying otherwise. Plus the EVEX and legacy cases are still
> >> being treated vastly different.
> >>
> >> Lingling, I notice you committed the patch with H.J.'s approval above,
> >> despite knowing there were open issues. I'm going to expect an
> >> incremental change to at least address the former of the two issues.
> >> Should that not arrive within a couple of days, I'm afraid I'm going to
> >> need to revert your change, for having been committed prematurely /
> >> unduly. You having done so is even more so odd because the requested
> >> adjustment would actually have simplified the code:
> >>
> >>             if (i.tm.mnem_off == MN_add
> >>                 && i.tm.opcode_space == SPACE_EVEXMAP4
> >>                 && i.mem_operands == 1
> >>                 && i.base_reg
> >>                 && i.base_reg->reg_num == RegIP
> >>                 && i.tm.operand_types[0].bitfield.class == Reg
> >>                 && i.tm.operand_types[i.operands - 1].bitfield.class ==
> >> Reg)
> >>
> >> For the latter of the two issues, if H.J. is unwilling to actually
> >> settle on consistent criteria between legacy and EVEX encodings, I guess
> >> it'll end up being me to actually make this code consistent, one way or
> >> another. The unwillingness to settle on criteria up front means that
> >> there then shall not be objections later on.
> >
> > Please open a bug report with a testcase.
>
> You're kidding? The inconsistency is blatantly obvious. And any testcase is
> going to be contrived anyway, as what we're discussing here is inconsistent
> application of an underlying, unwritten policy: How much is the assembler
> supposed to be refusing? How much control is the assembler supposed to be
> leaving to the programmer? I can live with such a policy being unwritten; I
> can't accept such a policy to be applied inconsistently (and hence
> unpredictably for the programmer).
>

TLS sequence is a special case.  It is hard to tell what your issues are
without a testcase.


> We did discuss all the same underlying issue with how you're approaching
> such things already when / after you introduced the entirely arbitrary KMOV
> special case (see your commit d7e3e627027f vs my later e3669c7f7ba4).
>
> Jan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240703/f16a80fd/attachment-0001.htm>