[PATCH] x86-64: Support APX NF TLS IE with 2 operands
Jan Beulich
jbeulich@suse.com
Wed Jul 3 08:10:22 GMT 2024
On 03.07.2024 09:58, Kong, Lingling wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Wednesday, July 3, 2024 3:28 PM
>>
>> 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)
>
> Hi,
>
> (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?
> Regarding your previous comment
>>> What does the i.tm.opcode_modifier.nf check achieve here? All
>>> EVexMap4 ADD forms permit {nf}. The comment also needs updating, to
>>> avoid it going further stale (it already hasn't been quite accurate).
>>> With the comment properly updated to list all permissible forms, I
>>> think you'll also note that what you add to the condition is too lax:
>>> Aiui
>>>
>>> add %rax, foo@GOTTPOFF(%rip)
>>>
>>> is not supposed to be permitted (according to the testsuite additions
>>> you make).
>
> For Aiui add %rax, foo@GOTTPOFF(%rip)
> Error: no such instruction: `aiui add %rax,foo@GOTTPOFF(%rip)',
Nice joke. Of course there's no insn "aiui". That also wasn't part of my
example.
> add %rax, foo@GOTTPOFF(%rip) is OK.
Yet it shouldn't be? The relocation is (apparently) meant to be permitted
only when the destination is a register?
> And you said all EVexMap4 ADD forms permit {nf} is yes, so I removed the restriction for {nf},
> EVexMap4 is enough. The code here just handle the TLS IE with EVEX encoding.
> Does not affect legacy instructions.
Then you didn't understand what I was saying. The problem is the very tight
restricting of APX EVEX encodings versus the very lax "all legacy encodings
are okay as long as they don't access a SIMD register".
Jan
More information about the Binutils
mailing list