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

[Jan Beulich]

On 04.07.2024 09:03, Kong, Lingling wrote:
> 
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Wednesday, July 3, 2024 10:11 PM
>> To: Kong, Lingling <lingling.kong@intel.com>
>> Cc: kong lingling <lingling.kong7@gmail.com>; Binutils
>> <binutils@sourceware.org>; H.J. Lu <hjl.tools@gmail.com>
>> Subject: Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
>>
>> On 03.07.2024 14:37, Kong, Lingling wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: Wednesday, July 3, 2024 4:10 PM
>>>>
>>>> On 03.07.2024 09:58, Kong, Lingling wrote:
>>>>> (i.tm.operand_types[2].bitfield.class == Reg || i.tm.operands == 2))
>>>>> and (i.tm.operand_types[i.operands -1].bitfield.class ==Reg) are not
>> equivalent.
>>>>> When i.operands = 2, i.tm.operand_types[1]. bitfield.class is not
>>>>> reg and should be mem,
>>>>
>>>> Funny you should say that: There's no class "mem". You have a point
>>>> though, but that's only indicating there are further issues here: It
>>>> shouldn't be the template that's being looked at, but the actual
>>>> operands. I.e. i.types[] rather than i.tm.operand_types[].
>>>>
>>>>> And it has been restricted i.mem_operands == 1 and i.tm.opcode_space
>>>>> ==
>>>> SPACE_EVEXMAP4 before.
>>>>
>>>> How does that help with a memory destination?
>>>
>>> i.tm.operand_types[0].bitfield.class == Reg want to restrict a register
>> destination.
>>
>> "Want to" != "does". If the template allows for both memory and (whatever kind
>> of) register, .class will indicate the register kind. You can tell whether the actual
>> operand is a register only from looking at i.types[] (or i.flags[])). Looking at
>> i.tm.operand_types[] only helps when memory isn't permitted there (and hence
>> operand matching would have failed when a memory operand is present in the
>> given operand position).
>>
>> Plus, btw, i.tm.operand_types[0] isn't describing the destination anyway, but (one
>> of) the source(s). Recall internal representation is following AT&T syntax.
> 
> OK, the restrictions should be more stringent with the following patch.
> diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
> index 3b4d9cacc2e..2382db23e19 100644
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -7544,9 +7544,8 @@ md_assemble (char *line)
>                 && 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[2].bitfield.class == Reg
> -                   || i.tm.operands == 2))
> +               && i.reg_operands == (i.operands - 1)
> +               && i.types[i.operands - 1].bitfield.class == Reg)
>               /* Allow APX:
>                  add %reg1, foo@gottpoff(%rip), %reg2
>                  add foo@gottpoff(%rip), %reg, %reg2

I'm okay with this, for the time being. Please put into proper patch form
(i.e. with a suitable description) and feel free to commit.

Jan