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

[H.J. Lu]

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

> On 03.07.2024 09:14, H.J. Lu wrote:
> > 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.
>
> I firmly explained what the issue with this is. Whereas you're firmly
> refusing
> to give an at least halfway appropriate response.
>

Have I missed something? I haven't seen any testcases.


> Jan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240703/207fd74c/attachment.htm>