* [PATCH] x86-64: Support APX NF TLS IE with 2 operands
@ 2024-07-03 1:19 kong lingling
2024-07-03 1:55 ` H.J. Lu
0 siblings, 1 reply; 22+ messages in thread
From: kong lingling @ 2024-07-03 1:19 UTC (permalink / raw)
To: Binutils, H.J. Lu; +Cc: Kong, Lingling
[-- Attachment #1: Type: text/plain, Size: 13445 bytes --]
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(-)
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 2e194313960..3b4d9cacc2e 100644
--- 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:
diff --git a/gas/testsuite/gas/i386/x86-64-gottpoff.d
b/gas/testsuite/gas/i386/x86-64-gottpoff.d
index 5d0dd582425..8f1c10f936c 100644
--- a/gas/testsuite/gas/i386/x86-64-gottpoff.d
+++ b/gas/testsuite/gas/i386/x86-64-gottpoff.d
@@ -24,4 +24,8 @@ Disassembly of section .text:
+[a-f0-9]+: 62 f4 9c 18 03 05 00 00 00 00 add
0x0\(%rip\),%rax,%r12 # 78 <_start\+0x78> 74:
R_X86_64_CODE_6_GOTTPOFF foo-0x4
+[a-f0-9]+: 62 74 fc 14 01 05 00 00 00 00 \{nf\} add
%r8,0x0\(%rip\),%r16 # 82 <_start\+0x82> 7e:
R_X86_64_CODE_6_GOTTPOFF foo-0x4
+[a-f0-9]+: 62 f4 9c 1c 03 05 00 00 00 00 \{nf\} add
0x0\(%rip\),%rax,%r12 # 8c <_start\+0x8c> 88:
R_X86_64_CODE_6_GOTTPOFF foo-0x4
+ +[a-f0-9]+: 62 f4 fc 0c 03 05 00 00 00 00 \{nf\} add
0x0\(%rip\),%rax # 96 <_start\+0x96> 92: R_X86_64_CODE_6_GOTTPOFF
foo-0x4
+ +[a-f0-9]+: 62 e4 fc 0c 03 05 00 00 00 00 \{nf\} add
0x0\(%rip\),%r16 # a0 <_start\+0xa0> 9c: R_X86_64_CODE_6_GOTTPOFF
foo-0x4
+ +[a-f0-9]+: 62 f4 fc 0c 03 05 00 00 00 00 \{nf\} add
0x0\(%rip\),%rax # aa <_start\+0xaa> a6: R_X86_64_CODE_6_GOTTPOFF
foo-0x4
+ +[a-f0-9]+: 62 e4 fc 0c 03 05 00 00 00 00 \{nf\} add
0x0\(%rip\),%r16 # b4 <_start\+0xb4> b0: R_X86_64_CODE_6_GOTTPOFF
foo-0x4
#pass
diff --git a/gas/testsuite/gas/i386/x86-64-gottpoff.s
b/gas/testsuite/gas/i386/x86-64-gottpoff.s
index d1f55017b0a..48d8a59a746 100644
--- a/gas/testsuite/gas/i386/x86-64-gottpoff.s
+++ b/gas/testsuite/gas/i386/x86-64-gottpoff.s
@@ -27,3 +27,13 @@ _start:
add r12, rax, QWORD PTR [rip + foo@GOTTPOFF]
{nf} add r16, QWORD PTR [rip + foo@GOTTPOFF], r8
{nf} add r12, rax, QWORD PTR [rip + foo@GOTTPOFF]
+
+ .att_syntax prefix
+
+ {nf} addq foo@GOTTPOFF(%rip), %rax
+ {nf} addq foo@GOTTPOFF(%rip), %r16
+
+ .intel_syntax noprefix
+
+ {nf} add rax, QWORD PTR [rip + foo@GOTTPOFF]
+ {nf} add r16, QWORD PTR [rip + foo@GOTTPOFF]
diff --git a/gold/testsuite/x86_64_ie_to_le.s
b/gold/testsuite/x86_64_ie_to_le.s
index 56f3dfcc35e..4bf3f4abafb 100644
--- a/gold/testsuite/x86_64_ie_to_le.s
+++ b/gold/testsuite/x86_64_ie_to_le.s
@@ -9,6 +9,7 @@ _start:
movq foo@gottpoff(%rip), %r20
addq %r30, foo@gottpoff(%rip), %r8
addq foo@gottpoff(%rip), %rax, %r20
+ {nf} addq foo@gottpoff(%rip), %r16
{nf} addq %r30, foo@gottpoff(%rip), %r8
{nf} addq foo@gottpoff(%rip), %rax, %r20
.size _start, .-_start
diff --git a/gold/testsuite/x86_64_ie_to_le.sh
b/gold/testsuite/x86_64_ie_to_le.sh
index 29272727c5a..10e84686db8 100755
--- a/gold/testsuite/x86_64_ie_to_le.sh
+++ b/gold/testsuite/x86_64_ie_to_le.sh
@@ -29,5 +29,6 @@ grep -q "add[ \t]\+\$0x[a-f0-9]\+,%r16"
x86_64_ie_to_le.stdout
grep -q "mov[ \t]\+\$0x[a-f0-9]\+,%r20" x86_64_ie_to_le.stdout
grep -q "add[ \t]\+\$0x[a-f0-9]\+,%r30,%r8" x86_64_ie_to_le.stdout
grep -q "add[ \t]\+\$0x[a-f0-9]\+,%rax,%r20" x86_64_ie_to_le.stdout
+grep -q "\{nf\} add[ \t]\+\$0x[a-f0-9]\+,%r16" x86_64_ie_to_le.stdout
grep -q "\{nf\} add[ \t]\+\$0x[a-f0-9]\+,%r30,%r8" x86_64_ie_to_le.stdout
grep -q "\{nf\} add[ \t]\+\$0x[a-f0-9]\+,%rax,%r20" x86_64_ie_to_le.stdout
diff --git a/ld/testsuite/ld-x86-64/tlsbindesc.dd
b/ld/testsuite/ld-x86-64/tlsbindesc.dd
index 7aa1b922b3c..601dfc2fc6d 100644
--- a/ld/testsuite/ld-x86-64/tlsbindesc.dd
+++ b/ld/testsuite/ld-x86-64/tlsbindesc.dd
@@ -173,6 +173,9 @@ Disassembly of section .text:
# -> R_X86_64_TPOFF64 sG2
+[0-9a-f]+: ([0-9a-f]{2} ){3} *
+[0-9a-f]+: 62 f4 fc 10 03 ([0-9a-f]{2} ){2}[ ]+add
0x[0-9a-f]+\(%rip\),%rax,%r16 +# [0-9a-f]+ <sG2>
+# -> R_X86_64_TPOFF64 sG2
+ +[0-9a-f]+: ([0-9a-f]{2} ){3} *
+ +[0-9a-f]+: 62 e4 fc 0c 03 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
0x[0-9a-f]+\(%rip\),%r16 +# [0-9a-f]+ <sG2>
# -> R_X86_64_TPOFF64 sG2
+[0-9a-f]+: ([0-9a-f]{2} ){3} *
+[0-9a-f]+: 62 f4 fc 14 01 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
%rax,0x[0-9a-f]+\(%rip\),%r16 +# [0-9a-f]+ <sG2>
@@ -189,6 +192,9 @@ Disassembly of section .text:
# sg1
+[0-9a-f]+: ff ff ff *
+[0-9a-f]+: 62 d4 f4 10 81 ([0-9a-f]{2} ){2}[ ]+add
\$0x[0-9a-f]+,%r8,%r17
+# sg1
+ +[0-9a-f]+: ff ff ff *
+ +[0-9a-f]+: 62 fc fc 0c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
\$0x[0-9a-f]+,%r17
# sg1
+[0-9a-f]+: ff ff ff *
+[0-9a-f]+: 62 d4 f4 14 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
\$0x[0-9a-f]+,%r8,%r17
@@ -205,6 +211,9 @@ Disassembly of section .text:
# sl1
+[0-9a-f]+: ff ff ff *
+[0-9a-f]+: 62 d4 fc 18 81 ([0-9a-f]{2} ){2}[ ]+add
\$0x[0-9a-f]+,%r8,%rax
+# sl1
+ +[0-9a-f]+: ff ff ff *
+ +[0-9a-f]+: 62 fc fc 0c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
\$0x[0-9a-f]+,%r18
# sl1
+[0-9a-f]+: ff ff ff *
+[0-9a-f]+: 62 d4 fc 1c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
\$0x[0-9a-f]+,%r8,%rax
@@ -221,6 +230,9 @@ Disassembly of section .text:
# sh1
+[0-9a-f]+: ff ff ff *
+[0-9a-f]+: 62 fc bc 18 81 ([0-9a-f]{2} ){2}[ ]+add
\$0x[0-9a-f]+,%r19,%r8
+# sh1
+ +[0-9a-f]+: ff ff ff *
+ +[0-9a-f]+: 62 fc fc 0c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
\$0x[0-9a-f]+,%r19
# sh1
+[0-9a-f]+: ff ff ff *
+[0-9a-f]+: 62 fc bc 1c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
\$0x[0-9a-f]+,%r19,%r8
diff --git a/ld/testsuite/ld-x86-64/tlsbindesc.rd
b/ld/testsuite/ld-x86-64/tlsbindesc.rd
index 1e0348e963f..2fc965aef20 100644
--- a/ld/testsuite/ld-x86-64/tlsbindesc.rd
+++ b/ld/testsuite/ld-x86-64/tlsbindesc.rd
@@ -15,12 +15,12 @@ Section Headers:
+\[[ 0-9]+\] .dynsym +.*
+\[[ 0-9]+\] .dynstr +.*
+\[[ 0-9]+\] .rela.dyn +.*
- +\[[ 0-9]+\] .text +PROGBITS +0+401000 0+1000 0+2fd 00 +AX +0 +0 +4096
- +\[[ 0-9]+\] .tdata +PROGBITS +0+6012fd 0+12fd 0+60 00 WAT +0 +0 +1
- +\[[ 0-9]+\] .tbss +NOBITS +0+60135d 0+135d 0+40 00 WAT +0 +0 +1
- +\[[ 0-9]+\] .dynamic +DYNAMIC +0+601360 0+1360 0+100 10 +WA +4 +0 +8
- +\[[ 0-9]+\] .got +PROGBITS +0+601460 0+1460 0+20 08 +WA +0 +0 +8
- +\[[ 0-9]+\] .got.plt +PROGBITS +0+601480 0+1480 0+18 08 +WA +0 +0 +8
+ +\[[ 0-9]+\] .text +PROGBITS +0+401000 0+1000 0+325 00 +AX +0 +0 +4096
+ +\[[ 0-9]+\] .tdata +PROGBITS +0+601325 0+1325 0+60 00 WAT +0 +0 +1
+ +\[[ 0-9]+\] .tbss +NOBITS +0+601385 0+1385 0+40 00 WAT +0 +0 +1
+ +\[[ 0-9]+\] .dynamic +DYNAMIC +0+601388 0+1388 0+100 10 +WA +4 +0 +8
+ +\[[ 0-9]+\] .got +PROGBITS +0+601488 0+1488 0+20 08 +WA +0 +0 +8
+ +\[[ 0-9]+\] .got.plt +PROGBITS +0+6014a8 0+14a8 0+18 08 +WA +0 +0 +8
+\[[ 0-9]+\] .symtab +.*
+\[[ 0-9]+\] .strtab +.*
+\[[ 0-9]+\] .shstrtab +.*
@@ -28,7 +28,7 @@ Key to Flags:
#...
Elf file type is EXEC \(Executable file\)
-Entry point 0x401205
+Entry point 0x40122d
There are [0-9]+ program headers, starting at offset [0-9]+
Program Headers:
@@ -36,10 +36,10 @@ Program Headers:
+PHDR.*
+INTERP.*
.*Requesting program interpreter.*
- +LOAD +0x0+ 0x0+400000 0x0+400000 0x0+12fd 0x0+12fd R E 0x200000
- +LOAD +0x0+12fd 0x0+6012fd 0x0+6012fd 0x0+19b 0x0+19b RW +0x200000
- +DYNAMIC +0x0+1360 0x0+601360 0x0+601360 0x0+100 0x0+100 RW +0x8
- +TLS +0x0+12fd 0x0+6012fd 0x0+6012fd 0x0+60 0x0+a0 R +0x1
+ +LOAD +0x0+ 0x0+400000 0x0+400000 0x0+1325 0x0+1325 R E 0x200000
+ +LOAD +0x0+1325 0x0+601325 0x0+601325 0x0+19b 0x0+19b RW +0x200000
+ +DYNAMIC +0x0+1388 0x0+601388 0x0+601388 0x0+100 0x0+100 RW +0x8
+ +TLS +0x0+1325 0x0+601325 0x0+601325 0x0+60 0x0+a0 R +0x1
Section to Segment mapping:
+Segment Sections...
@@ -52,10 +52,10 @@ Program Headers:
Relocation section '.rela.dyn' at offset 0x[0-9a-f]+ contains 4 entries:
+Offset +Info +Type +Symbol's Value +Symbol's Name \+ Addend
-0+601460 +0+100000012 R_X86_64_TPOFF64 +0+ sG5 \+ 0
-0+601468 +0+200000012 R_X86_64_TPOFF64 +0+ sG2 \+ 0
-0+601470 +0+300000012 R_X86_64_TPOFF64 +0+ sG6 \+ 0
-0+601478 +0+400000012 R_X86_64_TPOFF64 +0+ sG1 \+ 0
+0+601488 +0+100000012 R_X86_64_TPOFF64 +0+ sG5 \+ 0
+0+601490 +0+200000012 R_X86_64_TPOFF64 +0+ sG2 \+ 0
+0+601498 +0+300000012 R_X86_64_TPOFF64 +0+ sG6 \+ 0
+0+6014a0 +0+400000012 R_X86_64_TPOFF64 +0+ sG1 \+ 0
Symbol table '\.dynsym' contains [0-9]+ entries:
+Num: +Value +Size +Type +Bind +Vis +Ndx +Name
@@ -88,8 +88,8 @@ Symbol table '\.symtab' contains [0-9]+ entries:
+[0-9]+: 0+9c +0 +TLS +LOCAL +DEFAULT +8 bl8
.* FILE +LOCAL +DEFAULT +ABS
+[0-9]+: 0+a0 +0 +TLS +LOCAL +DEFAULT +7 _TLS_MODULE_BASE_
- +[0-9]+: 0+601360 +0 +OBJECT +LOCAL +DEFAULT +9 _DYNAMIC
- +[0-9]+: 0+601480 +0 +OBJECT +LOCAL +DEFAULT +11 _GLOBAL_OFFSET_TABLE_
+ +[0-9]+: 0+601388 +0 +OBJECT +LOCAL +DEFAULT +9 _DYNAMIC
+ +[0-9]+: 0+6014a8 +0 +OBJECT +LOCAL +DEFAULT +11 _GLOBAL_OFFSET_TABLE_
+[0-9]+: 0+1c +0 +TLS +GLOBAL +DEFAULT +7 sg8
+[0-9]+: 0+7c +0 +TLS +GLOBAL +DEFAULT +8 bg8
+[0-9]+: 0+74 +0 +TLS +GLOBAL +DEFAULT +8 bg6
@@ -104,7 +104,7 @@ Symbol table '\.symtab' contains [0-9]+ entries:
+[0-9]+: 0+58 +0 +TLS +GLOBAL +HIDDEN +7 sh7
+[0-9]+: 0+5c +0 +TLS +GLOBAL +HIDDEN +7 sh8
+[0-9]+: 0+ +0 +TLS +GLOBAL +DEFAULT +7 sg1
- +[0-9]+: 0+401205 +0 +FUNC +GLOBAL +DEFAULT +6 _start
+ +[0-9]+: 0+40122d +0 +FUNC +GLOBAL +DEFAULT +6 _start
+[0-9]+: 0+4c +0 +TLS +GLOBAL +HIDDEN +7 sh4
+[0-9]+: 0+78 +0 +TLS +GLOBAL +DEFAULT +8 bg7
+[0-9]+: 0+50 +0 +TLS +GLOBAL +HIDDEN +7 sh5
diff --git a/ld/testsuite/ld-x86-64/tlsbindesc.s
b/ld/testsuite/ld-x86-64/tlsbindesc.s
index 8aad1025de8..39ba3bbaa9f 100644
--- a/ld/testsuite/ld-x86-64/tlsbindesc.s
+++ b/ld/testsuite/ld-x86-64/tlsbindesc.s
@@ -131,6 +131,7 @@ fn2:
addq sG2@gottpoff(%rip), %r16
addq %rax, sG2@gottpoff(%rip), %r16
addq sG2@gottpoff(%rip), %rax, %r16
+ {nf} addq sG2@gottpoff(%rip), %r16
{nf} addq %rax, sG2@gottpoff(%rip), %r16
{nf} addq sG2@gottpoff(%rip), %rax, %r16
@@ -138,6 +139,7 @@ fn2:
addq sg1@gottpoff(%rip), %r17
addq %r8, sg1@gottpoff(%rip), %r17
addq sg1@gottpoff(%rip), %r8, %r17
+ {nf} addq sg1@gottpoff(%rip), %r17
{nf} addq %r8, sg1@gottpoff(%rip), %r17
{nf} addq sg1@gottpoff(%rip), %r8, %r17
@@ -145,6 +147,7 @@ fn2:
addq sl1@gottpoff(%rip), %r18
addq %r8, sl1@gottpoff(%rip), %rax
addq sl1@gottpoff(%rip), %r8, %rax
+ {nf} addq sl1@gottpoff(%rip), %r18
{nf} addq %r8, sl1@gottpoff(%rip), %rax
{nf} addq sl1@gottpoff(%rip), %r8, %rax
@@ -152,6 +155,7 @@ fn2:
addq sh1@gottpoff(%rip), %r19
addq %r19, sh1@gottpoff(%rip), %r8
addq sh1@gottpoff(%rip), %r19, %r8
+ {nf} addq sh1@gottpoff(%rip), %r19
{nf} addq %r19, sh1@gottpoff(%rip), %r8
{nf} addq sh1@gottpoff(%rip), %r19, %r8
--
2.31.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 1:19 [PATCH] x86-64: Support APX NF TLS IE with 2 operands kong lingling
@ 2024-07-03 1:55 ` H.J. Lu
2024-07-03 6:43 ` Jan Beulich
0 siblings, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-07-03 1:55 UTC (permalink / raw)
To: kong lingling; +Cc: Binutils, Kong, Lingling
[-- Attachment #1: Type: text/plain, Size: 14463 bytes --]
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(-)
>
> diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
> index 2e194313960..3b4d9cacc2e 100644
> --- 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:
> diff --git a/gas/testsuite/gas/i386/x86-64-gottpoff.d
> b/gas/testsuite/gas/i386/x86-64-gottpoff.d
> index 5d0dd582425..8f1c10f936c 100644
> --- a/gas/testsuite/gas/i386/x86-64-gottpoff.d
> +++ b/gas/testsuite/gas/i386/x86-64-gottpoff.d
> @@ -24,4 +24,8 @@ Disassembly of section .text:
> +[a-f0-9]+: 62 f4 9c 18 03 05 00 00 00 00 add
> 0x0\(%rip\),%rax,%r12 # 78 <_start\+0x78> 74:
> R_X86_64_CODE_6_GOTTPOFF foo-0x4
> +[a-f0-9]+: 62 74 fc 14 01 05 00 00 00 00 \{nf\} add
> %r8,0x0\(%rip\),%r16 # 82 <_start\+0x82> 7e:
> R_X86_64_CODE_6_GOTTPOFF foo-0x4
> +[a-f0-9]+: 62 f4 9c 1c 03 05 00 00 00 00 \{nf\} add
> 0x0\(%rip\),%rax,%r12 # 8c <_start\+0x8c> 88:
> R_X86_64_CODE_6_GOTTPOFF foo-0x4
> + +[a-f0-9]+: 62 f4 fc 0c 03 05 00 00 00 00 \{nf\} add
> 0x0\(%rip\),%rax # 96 <_start\+0x96> 92: R_X86_64_CODE_6_GOTTPOFF
> foo-0x4
> + +[a-f0-9]+: 62 e4 fc 0c 03 05 00 00 00 00 \{nf\} add
> 0x0\(%rip\),%r16 # a0 <_start\+0xa0> 9c: R_X86_64_CODE_6_GOTTPOFF
> foo-0x4
> + +[a-f0-9]+: 62 f4 fc 0c 03 05 00 00 00 00 \{nf\} add
> 0x0\(%rip\),%rax # aa <_start\+0xaa> a6: R_X86_64_CODE_6_GOTTPOFF
> foo-0x4
> + +[a-f0-9]+: 62 e4 fc 0c 03 05 00 00 00 00 \{nf\} add
> 0x0\(%rip\),%r16 # b4 <_start\+0xb4> b0: R_X86_64_CODE_6_GOTTPOFF
> foo-0x4
> #pass
> diff --git a/gas/testsuite/gas/i386/x86-64-gottpoff.s
> b/gas/testsuite/gas/i386/x86-64-gottpoff.s
> index d1f55017b0a..48d8a59a746 100644
> --- a/gas/testsuite/gas/i386/x86-64-gottpoff.s
> +++ b/gas/testsuite/gas/i386/x86-64-gottpoff.s
> @@ -27,3 +27,13 @@ _start:
> add r12, rax, QWORD PTR [rip + foo@GOTTPOFF]
> {nf} add r16, QWORD PTR [rip + foo@GOTTPOFF], r8
> {nf} add r12, rax, QWORD PTR [rip + foo@GOTTPOFF]
> +
> + .att_syntax prefix
> +
> + {nf} addq foo@GOTTPOFF(%rip), %rax
> + {nf} addq foo@GOTTPOFF(%rip), %r16
> +
> + .intel_syntax noprefix
> +
> + {nf} add rax, QWORD PTR [rip + foo@GOTTPOFF]
> + {nf} add r16, QWORD PTR [rip + foo@GOTTPOFF]
> diff --git a/gold/testsuite/x86_64_ie_to_le.s
> b/gold/testsuite/x86_64_ie_to_le.s
> index 56f3dfcc35e..4bf3f4abafb 100644
> --- a/gold/testsuite/x86_64_ie_to_le.s
> +++ b/gold/testsuite/x86_64_ie_to_le.s
> @@ -9,6 +9,7 @@ _start:
> movq foo@gottpoff(%rip), %r20
> addq %r30, foo@gottpoff(%rip), %r8
> addq foo@gottpoff(%rip), %rax, %r20
> + {nf} addq foo@gottpoff(%rip), %r16
> {nf} addq %r30, foo@gottpoff(%rip), %r8
> {nf} addq foo@gottpoff(%rip), %rax, %r20
> .size _start, .-_start
> diff --git a/gold/testsuite/x86_64_ie_to_le.sh
> b/gold/testsuite/x86_64_ie_to_le.sh
> index 29272727c5a..10e84686db8 100755
> --- a/gold/testsuite/x86_64_ie_to_le.sh
> +++ b/gold/testsuite/x86_64_ie_to_le.sh
> @@ -29,5 +29,6 @@ grep -q "add[ \t]\+\$0x[a-f0-9]\+,%r16"
> x86_64_ie_to_le.stdout
> grep -q "mov[ \t]\+\$0x[a-f0-9]\+,%r20" x86_64_ie_to_le.stdout
> grep -q "add[ \t]\+\$0x[a-f0-9]\+,%r30,%r8" x86_64_ie_to_le.stdout
> grep -q "add[ \t]\+\$0x[a-f0-9]\+,%rax,%r20" x86_64_ie_to_le.stdout
> +grep -q "\{nf\} add[ \t]\+\$0x[a-f0-9]\+,%r16" x86_64_ie_to_le.stdout
> grep -q "\{nf\} add[ \t]\+\$0x[a-f0-9]\+,%r30,%r8" x86_64_ie_to_le.stdout
> grep -q "\{nf\} add[ \t]\+\$0x[a-f0-9]\+,%rax,%r20" x86_64_ie_to_le.stdout
> diff --git a/ld/testsuite/ld-x86-64/tlsbindesc.dd
> b/ld/testsuite/ld-x86-64/tlsbindesc.dd
> index 7aa1b922b3c..601dfc2fc6d 100644
> --- a/ld/testsuite/ld-x86-64/tlsbindesc.dd
> +++ b/ld/testsuite/ld-x86-64/tlsbindesc.dd
> @@ -173,6 +173,9 @@ Disassembly of section .text:
> # -> R_X86_64_TPOFF64 sG2
> +[0-9a-f]+: ([0-9a-f]{2} ){3} *
> +[0-9a-f]+: 62 f4 fc 10 03 ([0-9a-f]{2} ){2}[ ]+add
> 0x[0-9a-f]+\(%rip\),%rax,%r16 +# [0-9a-f]+ <sG2>
> +# -> R_X86_64_TPOFF64 sG2
> + +[0-9a-f]+: ([0-9a-f]{2} ){3} *
> + +[0-9a-f]+: 62 e4 fc 0c 03 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
> 0x[0-9a-f]+\(%rip\),%r16 +# [0-9a-f]+ <sG2>
> # -> R_X86_64_TPOFF64 sG2
> +[0-9a-f]+: ([0-9a-f]{2} ){3} *
> +[0-9a-f]+: 62 f4 fc 14 01 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
> %rax,0x[0-9a-f]+\(%rip\),%r16 +# [0-9a-f]+ <sG2>
> @@ -189,6 +192,9 @@ Disassembly of section .text:
> # sg1
> +[0-9a-f]+: ff ff ff *
> +[0-9a-f]+: 62 d4 f4 10 81 ([0-9a-f]{2} ){2}[ ]+add
> \$0x[0-9a-f]+,%r8,%r17
> +# sg1
> + +[0-9a-f]+: ff ff ff *
> + +[0-9a-f]+: 62 fc fc 0c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
> \$0x[0-9a-f]+,%r17
> # sg1
> +[0-9a-f]+: ff ff ff *
> +[0-9a-f]+: 62 d4 f4 14 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
> \$0x[0-9a-f]+,%r8,%r17
> @@ -205,6 +211,9 @@ Disassembly of section .text:
> # sl1
> +[0-9a-f]+: ff ff ff *
> +[0-9a-f]+: 62 d4 fc 18 81 ([0-9a-f]{2} ){2}[ ]+add
> \$0x[0-9a-f]+,%r8,%rax
> +# sl1
> + +[0-9a-f]+: ff ff ff *
> + +[0-9a-f]+: 62 fc fc 0c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
> \$0x[0-9a-f]+,%r18
> # sl1
> +[0-9a-f]+: ff ff ff *
> +[0-9a-f]+: 62 d4 fc 1c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
> \$0x[0-9a-f]+,%r8,%rax
> @@ -221,6 +230,9 @@ Disassembly of section .text:
> # sh1
> +[0-9a-f]+: ff ff ff *
> +[0-9a-f]+: 62 fc bc 18 81 ([0-9a-f]{2} ){2}[ ]+add
> \$0x[0-9a-f]+,%r19,%r8
> +# sh1
> + +[0-9a-f]+: ff ff ff *
> + +[0-9a-f]+: 62 fc fc 0c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
> \$0x[0-9a-f]+,%r19
> # sh1
> +[0-9a-f]+: ff ff ff *
> +[0-9a-f]+: 62 fc bc 1c 81 ([0-9a-f]{2} ){2}[ ]+\{nf\} add
> \$0x[0-9a-f]+,%r19,%r8
> diff --git a/ld/testsuite/ld-x86-64/tlsbindesc.rd
> b/ld/testsuite/ld-x86-64/tlsbindesc.rd
> index 1e0348e963f..2fc965aef20 100644
> --- a/ld/testsuite/ld-x86-64/tlsbindesc.rd
> +++ b/ld/testsuite/ld-x86-64/tlsbindesc.rd
> @@ -15,12 +15,12 @@ Section Headers:
> +\[[ 0-9]+\] .dynsym +.*
> +\[[ 0-9]+\] .dynstr +.*
> +\[[ 0-9]+\] .rela.dyn +.*
> - +\[[ 0-9]+\] .text +PROGBITS +0+401000 0+1000 0+2fd 00 +AX +0 +0 +4096
> - +\[[ 0-9]+\] .tdata +PROGBITS +0+6012fd 0+12fd 0+60 00 WAT +0 +0 +1
> - +\[[ 0-9]+\] .tbss +NOBITS +0+60135d 0+135d 0+40 00 WAT +0 +0 +1
> - +\[[ 0-9]+\] .dynamic +DYNAMIC +0+601360 0+1360 0+100 10 +WA +4 +0 +8
> - +\[[ 0-9]+\] .got +PROGBITS +0+601460 0+1460 0+20 08 +WA +0 +0 +8
> - +\[[ 0-9]+\] .got.plt +PROGBITS +0+601480 0+1480 0+18 08 +WA +0 +0 +8
> + +\[[ 0-9]+\] .text +PROGBITS +0+401000 0+1000 0+325 00 +AX +0 +0 +4096
> + +\[[ 0-9]+\] .tdata +PROGBITS +0+601325 0+1325 0+60 00 WAT +0 +0 +1
> + +\[[ 0-9]+\] .tbss +NOBITS +0+601385 0+1385 0+40 00 WAT +0 +0 +1
> + +\[[ 0-9]+\] .dynamic +DYNAMIC +0+601388 0+1388 0+100 10 +WA +4 +0 +8
> + +\[[ 0-9]+\] .got +PROGBITS +0+601488 0+1488 0+20 08 +WA +0 +0 +8
> + +\[[ 0-9]+\] .got.plt +PROGBITS +0+6014a8 0+14a8 0+18 08 +WA +0 +0 +8
> +\[[ 0-9]+\] .symtab +.*
> +\[[ 0-9]+\] .strtab +.*
> +\[[ 0-9]+\] .shstrtab +.*
> @@ -28,7 +28,7 @@ Key to Flags:
> #...
>
> Elf file type is EXEC \(Executable file\)
> -Entry point 0x401205
> +Entry point 0x40122d
> There are [0-9]+ program headers, starting at offset [0-9]+
>
> Program Headers:
> @@ -36,10 +36,10 @@ Program Headers:
> +PHDR.*
> +INTERP.*
> .*Requesting program interpreter.*
> - +LOAD +0x0+ 0x0+400000 0x0+400000 0x0+12fd 0x0+12fd R E 0x200000
> - +LOAD +0x0+12fd 0x0+6012fd 0x0+6012fd 0x0+19b 0x0+19b RW +0x200000
> - +DYNAMIC +0x0+1360 0x0+601360 0x0+601360 0x0+100 0x0+100 RW +0x8
> - +TLS +0x0+12fd 0x0+6012fd 0x0+6012fd 0x0+60 0x0+a0 R +0x1
> + +LOAD +0x0+ 0x0+400000 0x0+400000 0x0+1325 0x0+1325 R E 0x200000
> + +LOAD +0x0+1325 0x0+601325 0x0+601325 0x0+19b 0x0+19b RW +0x200000
> + +DYNAMIC +0x0+1388 0x0+601388 0x0+601388 0x0+100 0x0+100 RW +0x8
> + +TLS +0x0+1325 0x0+601325 0x0+601325 0x0+60 0x0+a0 R +0x1
>
> Section to Segment mapping:
> +Segment Sections...
> @@ -52,10 +52,10 @@ Program Headers:
>
> Relocation section '.rela.dyn' at offset 0x[0-9a-f]+ contains 4 entries:
> +Offset +Info +Type +Symbol's Value +Symbol's Name \+ Addend
> -0+601460 +0+100000012 R_X86_64_TPOFF64 +0+ sG5 \+ 0
> -0+601468 +0+200000012 R_X86_64_TPOFF64 +0+ sG2 \+ 0
> -0+601470 +0+300000012 R_X86_64_TPOFF64 +0+ sG6 \+ 0
> -0+601478 +0+400000012 R_X86_64_TPOFF64 +0+ sG1 \+ 0
> +0+601488 +0+100000012 R_X86_64_TPOFF64 +0+ sG5 \+ 0
> +0+601490 +0+200000012 R_X86_64_TPOFF64 +0+ sG2 \+ 0
> +0+601498 +0+300000012 R_X86_64_TPOFF64 +0+ sG6 \+ 0
> +0+6014a0 +0+400000012 R_X86_64_TPOFF64 +0+ sG1 \+ 0
>
> Symbol table '\.dynsym' contains [0-9]+ entries:
> +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
> @@ -88,8 +88,8 @@ Symbol table '\.symtab' contains [0-9]+ entries:
> +[0-9]+: 0+9c +0 +TLS +LOCAL +DEFAULT +8 bl8
> .* FILE +LOCAL +DEFAULT +ABS
> +[0-9]+: 0+a0 +0 +TLS +LOCAL +DEFAULT +7 _TLS_MODULE_BASE_
> - +[0-9]+: 0+601360 +0 +OBJECT +LOCAL +DEFAULT +9 _DYNAMIC
> - +[0-9]+: 0+601480 +0 +OBJECT +LOCAL +DEFAULT +11 _GLOBAL_OFFSET_TABLE_
> + +[0-9]+: 0+601388 +0 +OBJECT +LOCAL +DEFAULT +9 _DYNAMIC
> + +[0-9]+: 0+6014a8 +0 +OBJECT +LOCAL +DEFAULT +11 _GLOBAL_OFFSET_TABLE_
> +[0-9]+: 0+1c +0 +TLS +GLOBAL +DEFAULT +7 sg8
> +[0-9]+: 0+7c +0 +TLS +GLOBAL +DEFAULT +8 bg8
> +[0-9]+: 0+74 +0 +TLS +GLOBAL +DEFAULT +8 bg6
> @@ -104,7 +104,7 @@ Symbol table '\.symtab' contains [0-9]+ entries:
> +[0-9]+: 0+58 +0 +TLS +GLOBAL +HIDDEN +7 sh7
> +[0-9]+: 0+5c +0 +TLS +GLOBAL +HIDDEN +7 sh8
> +[0-9]+: 0+ +0 +TLS +GLOBAL +DEFAULT +7 sg1
> - +[0-9]+: 0+401205 +0 +FUNC +GLOBAL +DEFAULT +6 _start
> + +[0-9]+: 0+40122d +0 +FUNC +GLOBAL +DEFAULT +6 _start
> +[0-9]+: 0+4c +0 +TLS +GLOBAL +HIDDEN +7 sh4
> +[0-9]+: 0+78 +0 +TLS +GLOBAL +DEFAULT +8 bg7
> +[0-9]+: 0+50 +0 +TLS +GLOBAL +HIDDEN +7 sh5
> diff --git a/ld/testsuite/ld-x86-64/tlsbindesc.s
> b/ld/testsuite/ld-x86-64/tlsbindesc.s
> index 8aad1025de8..39ba3bbaa9f 100644
> --- a/ld/testsuite/ld-x86-64/tlsbindesc.s
> +++ b/ld/testsuite/ld-x86-64/tlsbindesc.s
> @@ -131,6 +131,7 @@ fn2:
> addq sG2@gottpoff(%rip), %r16
> addq %rax, sG2@gottpoff(%rip), %r16
> addq sG2@gottpoff(%rip), %rax, %r16
> + {nf} addq sG2@gottpoff(%rip), %r16
> {nf} addq %rax, sG2@gottpoff(%rip), %r16
> {nf} addq sG2@gottpoff(%rip), %rax, %r16
>
> @@ -138,6 +139,7 @@ fn2:
> addq sg1@gottpoff(%rip), %r17
> addq %r8, sg1@gottpoff(%rip), %r17
> addq sg1@gottpoff(%rip), %r8, %r17
> + {nf} addq sg1@gottpoff(%rip), %r17
> {nf} addq %r8, sg1@gottpoff(%rip), %r17
> {nf} addq sg1@gottpoff(%rip), %r8, %r17
>
> @@ -145,6 +147,7 @@ fn2:
> addq sl1@gottpoff(%rip), %r18
> addq %r8, sl1@gottpoff(%rip), %rax
> addq sl1@gottpoff(%rip), %r8, %rax
> + {nf} addq sl1@gottpoff(%rip), %r18
> {nf} addq %r8, sl1@gottpoff(%rip), %rax
> {nf} addq sl1@gottpoff(%rip), %r8, %rax
>
> @@ -152,6 +155,7 @@ fn2:
> addq sh1@gottpoff(%rip), %r19
> addq %r19, sh1@gottpoff(%rip), %r8
> addq sh1@gottpoff(%rip), %r19, %r8
> + {nf} addq sh1@gottpoff(%rip), %r19
> {nf} addq %r19, sh1@gottpoff(%rip), %r8
> {nf} addq sh1@gottpoff(%rip), %r19, %r8
>
> --
> 2.31.1
>
OK.
Thanks.
H.J.
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 1:55 ` H.J. Lu
@ 2024-07-03 6:43 ` Jan Beulich
2024-07-03 6:48 ` H.J. Lu
0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2024-07-03 6:43 UTC (permalink / raw)
To: H.J. Lu, kong lingling; +Cc: Binutils, Kong, Lingling
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 6:43 ` Jan Beulich
@ 2024-07-03 6:48 ` H.J. Lu
2024-07-03 7:10 ` Jan Beulich
0 siblings, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-07-03 6:48 UTC (permalink / raw)
To: Jan Beulich; +Cc: kong lingling, Binutils, Kong, Lingling
[-- Attachment #1: Type: text/plain, Size: 4234 bytes --]
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.
Thanks.
> Jan
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 6:48 ` H.J. Lu
@ 2024-07-03 7:10 ` Jan Beulich
2024-07-03 7:14 ` H.J. Lu
0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2024-07-03 7:10 UTC (permalink / raw)
To: H.J. Lu; +Cc: kong lingling, Binutils, Kong, Lingling
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).
We did discuss all the same underlying issue with how you're approaching
such things already when / after you introduced the entirely arbitrary KMOV
special case (see your commit d7e3e627027f vs my later e3669c7f7ba4).
Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 7:10 ` Jan Beulich
@ 2024-07-03 7:14 ` H.J. Lu
2024-07-03 7:27 ` Jan Beulich
0 siblings, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-07-03 7:14 UTC (permalink / raw)
To: Jan Beulich; +Cc: kong lingling, Binutils, Kong, Lingling
[-- Attachment #1: Type: text/plain, Size: 5399 bytes --]
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.
> We did discuss all the same underlying issue with how you're approaching
> such things already when / after you introduced the entirely arbitrary KMOV
> special case (see your commit d7e3e627027f vs my later e3669c7f7ba4).
>
> Jan
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 7:14 ` H.J. Lu
@ 2024-07-03 7:27 ` Jan Beulich
2024-07-03 7:49 ` H.J. Lu
2024-07-03 7:58 ` Kong, Lingling
0 siblings, 2 replies; 22+ messages in thread
From: Jan Beulich @ 2024-07-03 7:27 UTC (permalink / raw)
To: H.J. Lu; +Cc: kong lingling, Binutils, Kong, Lingling
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 7:27 ` Jan Beulich
@ 2024-07-03 7:49 ` H.J. Lu
2024-07-03 9:23 ` Jan Beulich
2024-07-03 7:58 ` Kong, Lingling
1 sibling, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-07-03 7:49 UTC (permalink / raw)
To: Jan Beulich; +Cc: kong lingling, Binutils, Kong, Lingling
[-- Attachment #1: Type: text/plain, Size: 5759 bytes --]
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
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 7:27 ` Jan Beulich
2024-07-03 7:49 ` H.J. Lu
@ 2024-07-03 7:58 ` Kong, Lingling
2024-07-03 8:10 ` Jan Beulich
1 sibling, 1 reply; 22+ messages in thread
From: Kong, Lingling @ 2024-07-03 7:58 UTC (permalink / raw)
To: Beulich, Jan, H.J. Lu; +Cc: kong lingling, Binutils
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Wednesday, July 3, 2024 3:28 PM
> To: H.J. Lu <hjl.tools@gmail.com>
> Cc: kong lingling <lingling.kong7@gmail.com>; Binutils
> <binutils@sourceware.org>; Kong, Lingling <lingling.kong@intel.com>
> Subject: Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
>
> 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,
And it has been restricted i.mem_operands == 1 and i.tm.opcode_space == SPACE_EVEXMAP4 before.
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)',
add %rax, foo@GOTTPOFF(%rip) is OK.
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.
Thanks,
Lingling
> >>>> 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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 7:58 ` Kong, Lingling
@ 2024-07-03 8:10 ` Jan Beulich
2024-07-03 12:37 ` Kong, Lingling
0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2024-07-03 8:10 UTC (permalink / raw)
To: Kong, Lingling; +Cc: kong lingling, Binutils, H.J. Lu
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 7:49 ` H.J. Lu
@ 2024-07-03 9:23 ` Jan Beulich
2024-07-03 9:43 ` H.J. Lu
2024-07-03 10:01 ` Hongyu Wang
0 siblings, 2 replies; 22+ messages in thread
From: Jan Beulich @ 2024-07-03 9:23 UTC (permalink / raw)
To: H.J. Lu; +Cc: kong lingling, Binutils, Kong, Lingling
On 03.07.2024 09:49, H.J. Lu wrote:
> Have I missed something? I haven't seen any testcases.
From my description you could have made one yourself, if you really didn't
believe what I was saying. Here you go:
.text
gottpoff:
.irp op, add, sub
\op foo@gottpoff(%rip), %rax
\op foo@gottpoff(%rip), %r16
\op foo@gottpoff(%rip), %rax, %rcx
\op %rax, foo@gottpoff(%rip), %rcx
{nf} \op foo@gottpoff(%rip), %rax
.endr
popcnt foo@gottpoff(%rip), %rax
popcnt foo@gottpoff(%rip), %r16
{nf} popcnt foo@gottpoff(%rip), %rax
adox foo@gottpoff(%rip), %rax
adox foo@gottpoff(%rip), %r16
Thank you for making me waste time on demonstrating an obvious issue to you.
Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 9:23 ` Jan Beulich
@ 2024-07-03 9:43 ` H.J. Lu
2024-07-03 10:11 ` Jan Beulich
2024-07-03 10:01 ` Hongyu Wang
1 sibling, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-07-03 9:43 UTC (permalink / raw)
To: Jan Beulich; +Cc: kong lingling, Binutils, Kong, Lingling
[-- Attachment #1: Type: text/plain, Size: 968 bytes --]
On Wed, Jul 3, 2024, 5:23 PM Jan Beulich <jbeulich@suse.com> wrote:
> On 03.07.2024 09:49, H.J. Lu wrote:
> > Have I missed something? I haven't seen any testcases.
>
> From my description you could have made one yourself, if you really didn't
> believe what I was saying. Here you go:
>
> .text
> gottpoff:
> .irp op, add, sub
> \op foo@gottpoff(%rip), %rax
> \op foo@gottpoff(%rip), %r16
> \op foo@gottpoff(%rip), %rax, %rcx
> \op %rax, foo@gottpoff(%rip), %rcx
> {nf} \op foo@gottpoff(%rip), %rax
> .endr
>
> popcnt foo@gottpoff(%rip), %rax
> popcnt foo@gottpoff(%rip), %r16
> {nf} popcnt foo@gottpoff(%rip), %rax
>
> adox foo@gottpoff(%rip), %rax
> adox foo@gottpoff(%rip), %r16
>
What are the issues?
> Thank you for making me waste time on demonstrating an obvious issue to
> you.
>
> Jan
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 9:23 ` Jan Beulich
2024-07-03 9:43 ` H.J. Lu
@ 2024-07-03 10:01 ` Hongyu Wang
2024-07-03 10:13 ` Jan Beulich
1 sibling, 1 reply; 22+ messages in thread
From: Hongyu Wang @ 2024-07-03 10:01 UTC (permalink / raw)
To: Jan Beulich; +Cc: H.J. Lu, kong lingling, Binutils, Kong, Lingling
Jan Beulich <jbeulich@suse.com> 于2024年7月3日周三 17:23写道：
>
> On 03.07.2024 09:49, H.J. Lu wrote:
> > Have I missed something? I haven't seen any testcases.
>
> From my description you could have made one yourself, if you really didn't
> believe what I was saying. Here you go:
>
> .text
> gottpoff:
> .irp op, add, sub
> \op foo@gottpoff(%rip), %rax
> \op foo@gottpoff(%rip), %r16
> \op foo@gottpoff(%rip), %rax, %rcx
> \op %rax, foo@gottpoff(%rip), %rcx
> {nf} \op foo@gottpoff(%rip), %rax
> .endr
>
> popcnt foo@gottpoff(%rip), %rax
> popcnt foo@gottpoff(%rip), %r16
> {nf} popcnt foo@gottpoff(%rip), %rax
>
> adox foo@gottpoff(%rip), %rax
> adox foo@gottpoff(%rip), %r16
>
These instructions are actually allowed with the newly added
relocation type R_X86_64_CODE_{5,6}_GOTTPOFF, but there is no chance
for compiler to generate such code in TLS sequence.
I'm not sure how the logic can be simplified, we still need to check
for each operands for the allowed case, while we can add more ops
other than just MN_add.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 9:43 ` H.J. Lu
@ 2024-07-03 10:11 ` Jan Beulich
2024-07-03 10:20 ` H.J. Lu
0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2024-07-03 10:11 UTC (permalink / raw)
To: H.J. Lu; +Cc: kong lingling, Binutils, Kong, Lingling
On 03.07.2024 11:43, H.J. Lu wrote:
> On Wed, Jul 3, 2024, 5:23 PM Jan Beulich <jbeulich@suse.com> wrote:
>
>> On 03.07.2024 09:49, H.J. Lu wrote:
>>> Have I missed something? I haven't seen any testcases.
>>
>> From my description you could have made one yourself, if you really didn't
>> believe what I was saying. Here you go:
>>
>> .text
>> gottpoff:
>> .irp op, add, sub
>> \op foo@gottpoff(%rip), %rax
>> \op foo@gottpoff(%rip), %r16
>> \op foo@gottpoff(%rip), %rax, %rcx
>> \op %rax, foo@gottpoff(%rip), %rcx
>> {nf} \op foo@gottpoff(%rip), %rax
>> .endr
>>
>> popcnt foo@gottpoff(%rip), %rax
>> popcnt foo@gottpoff(%rip), %r16
>> {nf} popcnt foo@gottpoff(%rip), %rax
>>
>> adox foo@gottpoff(%rip), %rax
>> adox foo@gottpoff(%rip), %r16
>
> What are the issues?
Can you just explain the resulting behavior please, as to where errors are
raised and where insns assemble without any diagnostic? Did you even try
to hand this to the assembler? Because if you did, didn't it occur to you
that the set of diagnostics you got is completely arbitrary?
Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 10:01 ` Hongyu Wang
@ 2024-07-03 10:13 ` Jan Beulich
0 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2024-07-03 10:13 UTC (permalink / raw)
To: Hongyu Wang; +Cc: H.J. Lu, kong lingling, Binutils, Kong, Lingling
On 03.07.2024 12:01, Hongyu Wang wrote:
> Jan Beulich <jbeulich@suse.com> 于2024年7月3日周三 17:23写道：
>>
>> On 03.07.2024 09:49, H.J. Lu wrote:
>>> Have I missed something? I haven't seen any testcases.
>>
>> From my description you could have made one yourself, if you really didn't
>> believe what I was saying. Here you go:
>>
>> .text
>> gottpoff:
>> .irp op, add, sub
>> \op foo@gottpoff(%rip), %rax
>> \op foo@gottpoff(%rip), %r16
>> \op foo@gottpoff(%rip), %rax, %rcx
>> \op %rax, foo@gottpoff(%rip), %rcx
>> {nf} \op foo@gottpoff(%rip), %rax
>> .endr
>>
>> popcnt foo@gottpoff(%rip), %rax
>> popcnt foo@gottpoff(%rip), %r16
>> {nf} popcnt foo@gottpoff(%rip), %rax
>>
>> adox foo@gottpoff(%rip), %rax
>> adox foo@gottpoff(%rip), %r16
>>
>
> These instructions are actually allowed with the newly added
> relocation type R_X86_64_CODE_{5,6}_GOTTPOFF, but there is no chance
> for compiler to generate such code in TLS sequence.
> I'm not sure how the logic can be simplified, we still need to check
> for each operands for the allowed case, while we can add more ops
> other than just MN_add.
Why would operands need checking? Why would they not need checking in
the legacy encoding case? My present plan is to simply permit anything
in EVEX map 4. Unless of course we want to tighten what's permitted as
legacy encodings.
Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 10:11 ` Jan Beulich
@ 2024-07-03 10:20 ` H.J. Lu
2024-07-03 10:42 ` Jan Beulich
0 siblings, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-07-03 10:20 UTC (permalink / raw)
To: Jan Beulich; +Cc: kong lingling, Binutils, Kong, Lingling
[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]
On Wed, Jul 3, 2024, 6:11 PM Jan Beulich <jbeulich@suse.com> wrote:
> On 03.07.2024 11:43, H.J. Lu wrote:
> > On Wed, Jul 3, 2024, 5:23 PM Jan Beulich <jbeulich@suse.com> wrote:
> >
> >> On 03.07.2024 09:49, H.J. Lu wrote:
> >>> Have I missed something? I haven't seen any testcases.
> >>
> >> From my description you could have made one yourself, if you really
> didn't
> >> believe what I was saying. Here you go:
> >>
> >> .text
> >> gottpoff:
> >> .irp op, add, sub
> >> \op foo@gottpoff(%rip), %rax
> >> \op foo@gottpoff(%rip), %r16
> >> \op foo@gottpoff(%rip), %rax, %rcx
> >> \op %rax, foo@gottpoff(%rip), %rcx
> >> {nf} \op foo@gottpoff(%rip), %rax
> >> .endr
> >>
> >> popcnt foo@gottpoff(%rip), %rax
> >> popcnt foo@gottpoff(%rip), %r16
> >> {nf} popcnt foo@gottpoff(%rip), %rax
> >>
> >> adox foo@gottpoff(%rip), %rax
> >> adox foo@gottpoff(%rip), %r16
> >
> > What are the issues?
>
> Can you just explain the resulting behavior please, as to where errors are
> raised and where insns assemble without any diagnostic? Did you even try
> to hand this to the assembler? Because if you did, didn't it occur to you
> that the set of diagnostics you got is completely arbitrary?
>
foo@gottpoff(%rip) was added for TLS IE with ADD and MOV. Any other usages
are undefined.
If clarifications are needed, please raise the issue at x86-64 psABI group.
> Jan
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 10:20 ` H.J. Lu
@ 2024-07-03 10:42 ` Jan Beulich
2024-07-03 10:51 ` H.J. Lu
0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2024-07-03 10:42 UTC (permalink / raw)
To: H.J. Lu; +Cc: kong lingling, Binutils, Kong, Lingling
On 03.07.2024 12:20, H.J. Lu wrote:
> On Wed, Jul 3, 2024, 6:11 PM Jan Beulich <jbeulich@suse.com> wrote:
>
>> On 03.07.2024 11:43, H.J. Lu wrote:
>>> On Wed, Jul 3, 2024, 5:23 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>>> On 03.07.2024 09:49, H.J. Lu wrote:
>>>>> Have I missed something? I haven't seen any testcases.
>>>>
>>>> From my description you could have made one yourself, if you really
>> didn't
>>>> believe what I was saying. Here you go:
>>>>
>>>> .text
>>>> gottpoff:
>>>> .irp op, add, sub
>>>> \op foo@gottpoff(%rip), %rax
>>>> \op foo@gottpoff(%rip), %r16
>>>> \op foo@gottpoff(%rip), %rax, %rcx
>>>> \op %rax, foo@gottpoff(%rip), %rcx
>>>> {nf} \op foo@gottpoff(%rip), %rax
>>>> .endr
>>>>
>>>> popcnt foo@gottpoff(%rip), %rax
>>>> popcnt foo@gottpoff(%rip), %r16
>>>> {nf} popcnt foo@gottpoff(%rip), %rax
>>>>
>>>> adox foo@gottpoff(%rip), %rax
>>>> adox foo@gottpoff(%rip), %r16
>>>
>>> What are the issues?
>>
>> Can you just explain the resulting behavior please, as to where errors are
>> raised and where insns assemble without any diagnostic? Did you even try
>> to hand this to the assembler? Because if you did, didn't it occur to you
>> that the set of diagnostics you got is completely arbitrary?
>>
>
> foo@gottpoff(%rip) was added for TLS IE with ADD and MOV. Any other usages
> are undefined.
Yet gas behaves inconsistently in this regard. It accepts (taking just the
example above) SUB, POPCNT, and ADOX, just as long as they're legacy
encoded.
> If clarifications are needed, please raise the issue at x86-64 psABI group.
I don't think clarification is needed there. We simply need to get gas into
a mode where it behaves consistently. That could be consistently more lax
or consistently more strict. I'm tending towards the former, as indicated.
Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 10:42 ` Jan Beulich
@ 2024-07-03 10:51 ` H.J. Lu
0 siblings, 0 replies; 22+ messages in thread
From: H.J. Lu @ 2024-07-03 10:51 UTC (permalink / raw)
To: Jan Beulich; +Cc: kong lingling, Binutils, Kong, Lingling
[-- Attachment #1: Type: text/plain, Size: 2362 bytes --]
On Wed, Jul 3, 2024, 6:42 PM Jan Beulich <jbeulich@suse.com> wrote:
> On 03.07.2024 12:20, H.J. Lu wrote:
> > On Wed, Jul 3, 2024, 6:11 PM Jan Beulich <jbeulich@suse.com> wrote:
> >
> >> On 03.07.2024 11:43, H.J. Lu wrote:
> >>> On Wed, Jul 3, 2024, 5:23 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>>
> >>>> On 03.07.2024 09:49, H.J. Lu wrote:
> >>>>> Have I missed something? I haven't seen any testcases.
> >>>>
> >>>> From my description you could have made one yourself, if you really
> >> didn't
> >>>> believe what I was saying. Here you go:
> >>>>
> >>>> .text
> >>>> gottpoff:
> >>>> .irp op, add, sub
> >>>> \op foo@gottpoff(%rip), %rax
> >>>> \op foo@gottpoff(%rip), %r16
> >>>> \op foo@gottpoff(%rip), %rax, %rcx
> >>>> \op %rax, foo@gottpoff(%rip), %rcx
> >>>> {nf} \op foo@gottpoff(%rip), %rax
> >>>> .endr
> >>>>
> >>>> popcnt foo@gottpoff(%rip), %rax
> >>>> popcnt foo@gottpoff(%rip), %r16
> >>>> {nf} popcnt foo@gottpoff(%rip), %rax
> >>>>
> >>>> adox foo@gottpoff(%rip), %rax
> >>>> adox foo@gottpoff(%rip), %r16
> >>>
> >>> What are the issues?
> >>
> >> Can you just explain the resulting behavior please, as to where errors
> are
> >> raised and where insns assemble without any diagnostic? Did you even try
> >> to hand this to the assembler? Because if you did, didn't it occur to
> you
> >> that the set of diagnostics you got is completely arbitrary?
> >>
> >
> > foo@gottpoff(%rip) was added for TLS IE with ADD and MOV. Any other
> usages
> > are undefined.
>
> Yet gas behaves inconsistently in this regard. It accepts (taking just the
> example above) SUB, POPCNT, and ADOX, just as long as they're legacy
> encoded.
>
> > If clarifications are needed, please raise the issue at x86-64 psABI
> group.
>
> I don't think clarification is needed there. We simply need to get gas into
> a mode where it behaves consistently. That could be consistently more lax
> or consistently more strict. I'm tending towards the former, as indicated.
>
The resulting relocation is checked by linker to rewrite ADD and MOV. I
didn't
check if linker will generate the corrupted output when the relocation is
used on
other instructions.
> Jan
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 8:10 ` Jan Beulich
@ 2024-07-03 12:37 ` Kong, Lingling
2024-07-03 14:11 ` Jan Beulich
0 siblings, 1 reply; 22+ messages in thread
From: Kong, Lingling @ 2024-07-03 12:37 UTC (permalink / raw)
To: Beulich, Jan; +Cc: kong lingling, Binutils, H.J. Lu
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Wednesday, July 3, 2024 4:10 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 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?
i.tm.operand_types[0].bitfield.class == Reg want to restrict a register 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.
Sorry, I misunderstood before.
> > 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?
Yes, for linker the relocation is (apparently) meant to be permitted only
when the destination is a register, but the opcode(0x8b or 0x03) is detected in the linker.
But the gas for legacy encoding do not check.
> > 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".
My patch just supported and listed all TLS IE with EVEX encoding which these TLS sequence also supported in linker.
The problem you mentioned does exist. It feels like a historical issue for undefined behavior. According to your
expectations, either the gas for EVEX encoding should be more relaxed , or more strict for all legacy instructions.
I don't know if I understand it correctly, although I prefer the latter. If you prefer the former, you can indeed modify
this code.
> Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 12:37 ` Kong, Lingling
@ 2024-07-03 14:11 ` Jan Beulich
2024-07-04 7:03 ` Kong, Lingling
0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2024-07-03 14:11 UTC (permalink / raw)
To: Kong, Lingling; +Cc: kong lingling, Binutils, H.J. Lu
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.
>>> 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".
>
> My patch just supported and listed all TLS IE with EVEX encoding which these TLS sequence also supported in linker.
> The problem you mentioned does exist. It feels like a historical issue for undefined behavior. According to your
> expectations, either the gas for EVEX encoding should be more relaxed , or more strict for all legacy instructions.
> I don't know if I understand it correctly, although I prefer the latter. If you prefer the former, you can indeed modify
> this code.
Well. If it ends up being me to make this more consistent, I'd be relaxing
it. Unless of course good reasons are brought forward of why relaxing is a
bad idea. This is on the grounds of the assembler not being supposed to
needlessly restrict people in what they're doing. We should only reject
code if we _know_ it can't work, or if from what we get to see we can't
really conclude what is meant. This way we allow creative people to write
code we didn't even think about could be written.
If otoh somebody else, e.g. you, was taking care of the issue, then I also
wouldn't mind tightening, as long as proper justification is being provided.
Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-03 14:11 ` Jan Beulich
@ 2024-07-04 7:03 ` Kong, Lingling
2024-07-04 7:38 ` Jan Beulich
0 siblings, 1 reply; 22+ messages in thread
From: Kong, Lingling @ 2024-07-04 7:03 UTC (permalink / raw)
To: Beulich, Jan; +Cc: kong lingling, Binutils, H.J. Lu
> -----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
> >>> 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".
> >
> > My patch just supported and listed all TLS IE with EVEX encoding which these
> TLS sequence also supported in linker.
> > The problem you mentioned does exist. It feels like a historical issue
> > for undefined behavior. According to your expectations, either the gas for EVEX
> encoding should be more relaxed , or more strict for all legacy instructions.
> > I don't know if I understand it correctly, although I prefer the
> > latter. If you prefer the former, you can indeed modify this code.
>
> Well. If it ends up being me to make this more consistent, I'd be relaxing it. Unless
> of course good reasons are brought forward of why relaxing is a bad idea. This is
> on the grounds of the assembler not being supposed to needlessly restrict people
> in what they're doing. We should only reject code if we _know_ it can't work, or if
> from what we get to see we can't really conclude what is meant. This way we
> allow creative people to write code we didn't even think about could be written.
>
> If otoh somebody else, e.g. you, was taking care of the issue, then I also wouldn't
> mind tightening, as long as proper justification is being provided.
>
> Jan
Ok, if you choose to relax, you can revert my patch or modify this part directly.
Thanks,
Lingling
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64: Support APX NF TLS IE with 2 operands
2024-07-04 7:03 ` Kong, Lingling
@ 2024-07-04 7:38 ` Jan Beulich
0 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2024-07-04 7:38 UTC (permalink / raw)
To: Kong, Lingling; +Cc: kong lingling, Binutils, H.J. Lu
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
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2024-07-04 7:38 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-03 1:19 [PATCH] x86-64: Support APX NF TLS IE with 2 operands kong lingling
2024-07-03 1:55 ` H.J. Lu
2024-07-03 6:43 ` Jan Beulich
2024-07-03 6:48 ` H.J. Lu
2024-07-03 7:10 ` Jan Beulich
2024-07-03 7:14 ` H.J. Lu
2024-07-03 7:27 ` Jan Beulich
2024-07-03 7:49 ` H.J. Lu
2024-07-03 9:23 ` Jan Beulich
2024-07-03 9:43 ` H.J. Lu
2024-07-03 10:11 ` Jan Beulich
2024-07-03 10:20 ` H.J. Lu
2024-07-03 10:42 ` Jan Beulich
2024-07-03 10:51 ` H.J. Lu
2024-07-03 10:01 ` Hongyu Wang
2024-07-03 10:13 ` Jan Beulich
2024-07-03 7:58 ` Kong, Lingling
2024-07-03 8:10 ` Jan Beulich
2024-07-03 12:37 ` Kong, Lingling
2024-07-03 14:11 ` Jan Beulich
2024-07-04 7:03 ` Kong, Lingling
2024-07-04 7:38 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).