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

[Jan Beulich]

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.

Jan