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

[Jan Beulich]

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.

Jan