[v5,8/8] x86: further re-work insn/suffix recognition to also cover MOVSX

Message ID 06ff83d4-4633-a07b-70e5-a8e049981dd4@suse.com
State Accepted
Headers
Series x86: suffix handling changes |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Jan Beulich Oct. 25, 2022, 7:29 a.m. UTC
  PR gas/29524

Having templates with a suffix explicitly present has always been
quirky. After prior adjustment all that's left to also eliminate the
anomaly from move-with-sign-extend is to consolidate the insn templates
(and extend testsuite coverage).
---
v5: Split off from earlier patch, merged with prior patch dealing with
    just x86-64's MOVSL.
  

Comments

H.J. Lu Oct. 25, 2022, 5:10 p.m. UTC | #1
On Tue, Oct 25, 2022 at 12:29 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> PR gas/29524
>
> Having templates with a suffix explicitly present has always been
> quirky. After prior adjustment all that's left to also eliminate the

I prefer keeping the current MOVS behavior.  Please submit a new patch
set without MOVS changes.

> anomaly from move-with-sign-extend is to consolidate the insn templates
> (and extend testsuite coverage).
> ---
> v5: Split off from earlier patch, merged with prior patch dealing with
>     just x86-64's MOVSL.
>
> --- a/gas/testsuite/gas/i386/i386.exp
> +++ b/gas/testsuite/gas/i386/i386.exp
> @@ -73,6 +73,7 @@ if [gas_32_check] then {
>      run_dump_test "amd"
>      run_dump_test "katmai"
>      run_dump_test "jump"
> +    run_dump_test "movs32"
>      run_dump_test "movz32"
>      run_dump_test "relax-1"
>      run_dump_test "relax-2"
> @@ -804,6 +805,7 @@ if [gas_64_check] then {
>      run_dump_test "x86-64-segovr"
>      run_list_test "x86-64-inval-seg" "-al"
>      run_dump_test "x86-64-branch"
> +    run_dump_test "movs64"
>      run_dump_test "movz64"
>      run_dump_test "x86-64-relax-1"
>      run_dump_test "svme64"
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/movs.s
> @@ -0,0 +1,39 @@
> +       .text
> +movs:
> +       movsb   %al,%ax
> +       movsb   (%eax),%ax
> +       movsb   %al,%eax
> +       movsb   (%eax),%eax
> +.ifdef x86_64
> +       movsb   %al,%rax
> +       movsb   (%rax),%rax
> +.endif
> +
> +       movsbw  %al,%ax
> +       movsbw  (%eax),%ax
> +       movsbl  %al,%eax
> +       movsbl  (%eax),%eax
> +.ifdef x86_64
> +       movsbq  %al,%rax
> +       movsbq  (%rax),%rax
> +.endif
> +
> +       movsw   %ax,%eax
> +       movsw   (%eax),%eax
> +.ifdef x86_64
> +       movsw   %ax,%rax
> +       movsw   (%rax),%rax
> +.endif
> +
> +       movswl  %ax,%eax
> +       movswl  (%eax),%eax
> +.ifdef x86_64
> +       movswq  %ax,%rax
> +       movswq  (%rax),%rax
> +
> +       movsl   %eax,%rax
> +       movsl   (%rax),%rax
> +
> +       movslq  %eax,%rax
> +       movslq  (%rax),%rax
> +.endif
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/movs32.d
> @@ -0,0 +1,22 @@
> +#objdump: -dw
> +#source: movs.s
> +#name: x86 mov with sign-extend (32-bit object)
> +
> +.*: +file format .*
> +
> +Disassembly of section .text:
> +
> +0+ <movs>:
> +[      ]*[a-f0-9]+:    66 0f be c0 *   movsbw %al,%ax
> +[      ]*[a-f0-9]+:    66 0f be 00 *   movsbw \(%eax\),%ax
> +[      ]*[a-f0-9]+:    0f be c0 *      movsbl %al,%eax
> +[      ]*[a-f0-9]+:    0f be 00 *      movsbl \(%eax\),%eax
> +[      ]*[a-f0-9]+:    66 0f be c0 *   movsbw %al,%ax
> +[      ]*[a-f0-9]+:    66 0f be 00 *   movsbw \(%eax\),%ax
> +[      ]*[a-f0-9]+:    0f be c0 *      movsbl %al,%eax
> +[      ]*[a-f0-9]+:    0f be 00 *      movsbl \(%eax\),%eax
> +[      ]*[a-f0-9]+:    0f bf c0 *      movswl %ax,%eax
> +[      ]*[a-f0-9]+:    0f bf 00 *      movswl \(%eax\),%eax
> +[      ]*[a-f0-9]+:    0f bf c0 *      movswl %ax,%eax
> +[      ]*[a-f0-9]+:    0f bf 00 *      movswl \(%eax\),%eax
> +#pass
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/movs64.d
> @@ -0,0 +1,30 @@
> +#objdump: -dw
> +#source: movs.s
> +#name: x86 mov with sign-extend (64-bit object)
> +
> +.*: +file format .*
> +
> +Disassembly of section .text:
> +
> +0+ <movs>:
> +[      ]*[a-f0-9]+:    66 0f be c0 *   movsbw %al,%ax
> +[      ]*[a-f0-9]+:    67 66 0f be 00 *        movsbw \(%eax\),%ax
> +[      ]*[a-f0-9]+:    0f be c0 *      movsbl %al,%eax
> +[      ]*[a-f0-9]+:    67 0f be 00 *   movsbl \(%eax\),%eax
> +[      ]*[a-f0-9]+:    48 0f be c0 *   movsbq %al,%rax
> +[      ]*[a-f0-9]+:    48 0f be 00 *   movsbq \(%rax\),%rax
> +[      ]*[a-f0-9]+:    66 0f be c0 *   movsbw %al,%ax
> +[      ]*[a-f0-9]+:    67 66 0f be 00 *        movsbw \(%eax\),%ax
> +[      ]*[a-f0-9]+:    0f be c0 *      movsbl %al,%eax
> +[      ]*[a-f0-9]+:    67 0f be 00 *   movsbl \(%eax\),%eax
> +[      ]*[a-f0-9]+:    48 0f be c0 *   movsbq %al,%rax
> +[      ]*[a-f0-9]+:    48 0f be 00 *   movsbq \(%rax\),%rax
> +[      ]*[a-f0-9]+:    0f bf c0 *      movswl %ax,%eax
> +[      ]*[a-f0-9]+:    67 0f bf 00 *   movswl \(%eax\),%eax
> +[      ]*[a-f0-9]+:    48 0f bf c0 *   movswq %ax,%rax
> +[      ]*[a-f0-9]+:    48 0f bf 00 *   movswq \(%rax\),%rax
> +[      ]*[a-f0-9]+:    0f bf c0 *      movswl %ax,%eax
> +[      ]*[a-f0-9]+:    67 0f bf 00 *   movswl \(%eax\),%eax
> +[      ]*[a-f0-9]+:    48 0f bf c0 *   movswq %ax,%rax
> +[      ]*[a-f0-9]+:    48 0f bf 00 *   movswq \(%rax\),%rax
> +#pass
> --- a/gas/testsuite/gas/i386/movx16.l
> +++ b/gas/testsuite/gas/i386/movx16.l
> @@ -41,11 +41,11 @@
>  [      ]*[1-9][0-9]*[  ]+movsb %ax, %cl
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %cl
>  [      ]*[1-9][0-9]*[  ]*
> -[      ]*[1-9][0-9]*[  ]+movsb %al, %cx
> +[      ]*[1-9][0-9]* \?\?\?\? 0FBEC8[  ]+movsb %al, %cx
>  [      ]*[1-9][0-9]*[  ]+movsb %ax, %cx
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %cx
>  [      ]*[1-9][0-9]*[  ]*
> -[      ]*[1-9][0-9]*[  ]+movsb %al, %ecx
> +[      ]*[1-9][0-9]* \?\?\?\? 660FBEC8[        ]+movsb %al, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsb %ax, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %ecx
>  [      ]*[1-9][0-9]*[  ]*
> @@ -82,7 +82,7 @@
>  [      ]*[1-9][0-9]*[  ]+movsw %eax, %cx
>  [      ]*[1-9][0-9]*[  ]*
>  [      ]*[1-9][0-9]*[  ]+movsw %al, %ecx
> -[      ]*[1-9][0-9]*[  ]+movsw %ax, %ecx
> +[      ]*[1-9][0-9]* \?\?\?\? 660FBFC8[        ]+movsw %ax, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsw %eax, %ecx
>  [      ]*[1-9][0-9]*[  ]*
>  [      ]*[1-9][0-9]*[  ]+movswl        %al, %cl
> --- a/gas/testsuite/gas/i386/movx32.l
> +++ b/gas/testsuite/gas/i386/movx32.l
> @@ -41,11 +41,11 @@
>  [      ]*[1-9][0-9]*[  ]+movsb %ax, %cl
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %cl
>  [      ]*[1-9][0-9]*[  ]*
> -[      ]*[1-9][0-9]*[  ]+movsb %al, %cx
> +[      ]*[1-9][0-9]* \?\?\?\? 660FBEC8[        ]+movsb %al, %cx
>  [      ]*[1-9][0-9]*[  ]+movsb %ax, %cx
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %cx
>  [      ]*[1-9][0-9]*[  ]*
> -[      ]*[1-9][0-9]*[  ]+movsb %al, %ecx
> +[      ]*[1-9][0-9]* \?\?\?\? 0FBEC8[  ]+movsb %al, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsb %ax, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %ecx
>  [      ]*[1-9][0-9]*[  ]*
> @@ -82,7 +82,7 @@
>  [      ]*[1-9][0-9]*[  ]+movsw %eax, %cx
>  [      ]*[1-9][0-9]*[  ]*
>  [      ]*[1-9][0-9]*[  ]+movsw %al, %ecx
> -[      ]*[1-9][0-9]*[  ]+movsw %ax, %ecx
> +[      ]*[1-9][0-9]* \?\?\?\? 0FBFC8[  ]+movsw %ax, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsw %eax, %ecx
>  [      ]*[1-9][0-9]*[  ]*
>  [      ]*[1-9][0-9]*[  ]+movswl        %al, %cl
> --- a/gas/testsuite/gas/i386/movx64.l
> +++ b/gas/testsuite/gas/i386/movx64.l
> @@ -106,17 +106,17 @@
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %cl
>  [      ]*[1-9][0-9]*[  ]+movsb %rax, %cl
>  [      ]*[1-9][0-9]*[  ]*
> -[      ]*[1-9][0-9]*[  ]+movsb %al, %cx
> +[      ]*[1-9][0-9]* \?\?\?\? 660FBEC8[        ]+movsb %al, %cx
>  [      ]*[1-9][0-9]*[  ]+movsb %ax, %cx
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %cx
>  [      ]*[1-9][0-9]*[  ]+movsb %rax, %cx
>  [      ]*[1-9][0-9]*[  ]*
> -[      ]*[1-9][0-9]*[  ]+movsb %al, %ecx
> +[      ]*[1-9][0-9]* \?\?\?\? 0FBEC8[  ]+movsb %al, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsb %ax, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsb %rax, %ecx
>  [      ]*[1-9][0-9]*[  ]*
> -[      ]*[1-9][0-9]*[  ]+movsb %al, %rcx
> +[      ]*[1-9][0-9]* \?\?\?\? 480FBEC8[        ]+movsb %al, %rcx
>  [      ]*[1-9][0-9]*[  ]+movsb %ax, %rcx
>  [      ]*[1-9][0-9]*[  ]+movsb %eax, %rcx
>  [      ]*[1-9][0-9]*[  ]+movsb %rax, %rcx
> @@ -192,12 +192,12 @@
>  [      ]*[1-9][0-9]*[  ]+movsw %rax, %cx
>  [      ]*[1-9][0-9]*[  ]*
>  [      ]*[1-9][0-9]*[  ]+movsw %al, %ecx
> -[      ]*[1-9][0-9]*[  ]+movsw %ax, %ecx
> +[      ]*[1-9][0-9]* \?\?\?\? 0FBFC8[  ]+movsw %ax, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsw %eax, %ecx
>  [      ]*[1-9][0-9]*[  ]+movsw %rax, %ecx
>  [      ]*[1-9][0-9]*[  ]*
>  [      ]*[1-9][0-9]*[  ]+movsw %al, %rcx
> -[      ]*[1-9][0-9]*[  ]+movsw %ax, %rcx
> +[      ]*[1-9][0-9]* \?\?\?\? 480FBFC8[        ]+movsw %ax, %rcx
>  [      ]*[1-9][0-9]*[  ]+movsw %eax, %rcx
>  [      ]*[1-9][0-9]*[  ]+movsw %rax, %rcx
>  [      ]*[1-9][0-9]*[  ]*
> @@ -241,6 +241,46 @@
>  [      ]*[1-9][0-9]*[  ]+movswq        %eax, %rcx
>  [      ]*[1-9][0-9]*[  ]+movswq        %rax, %rcx
>  [      ]*[1-9][0-9]*[  ]*
> +[      ]*[1-9][0-9]*[  ]+movsl %al, %cl
> +[      ]*[1-9][0-9]*[  ]+movsl %ax, %cl
> +[      ]*[1-9][0-9]*[  ]+movsl %eax, %cl
> +[      ]*[1-9][0-9]*[  ]+movsl %rax, %cl
> +[      ]*[1-9][0-9]*[  ]*
> +[      ]*[1-9][0-9]*[  ]+movsl %al, %cx
> +[      ]*[1-9][0-9]*[  ]+movsl %ax, %cx
> +[      ]*[1-9][0-9]*[  ]+movsl %eax, %cx
> +[      ]*[1-9][0-9]*[  ]+movsl %rax, %cx
> +[      ]*[1-9][0-9]*[  ]*
> +[      ]*[1-9][0-9]*[  ]+movsl %al, %ecx
> +[      ]*[1-9][0-9]*[  ]+movsl %ax, %ecx
> +[      ]*[1-9][0-9]*[  ]+movsl %eax, %ecx
> +[      ]*[1-9][0-9]*[  ]+movsl %rax, %ecx
> +[      ]*[1-9][0-9]*[  ]*
> +[      ]*[1-9][0-9]*[  ]+movsl %al, %rcx
> +[      ]*[1-9][0-9]*[  ]+movsl %ax, %rcx
> +[      ]*[1-9][0-9]* \?\?\?\? 4863C8[  ]+movsl %eax, %rcx
> +[      ]*[1-9][0-9]*[  ]+movsl %rax, %rcx
> +[      ]*[1-9][0-9]*[  ]*
> +[      ]*[1-9][0-9]*[  ]+movslq        %al, %cl
> +[      ]*[1-9][0-9]*[  ]+movslq        %ax, %cl
> +[      ]*[1-9][0-9]*[  ]+movslq        %eax, %cl
> +[      ]*[1-9][0-9]*[  ]+movslq        %rax, %cl
> +[      ]*[1-9][0-9]*[  ]*
> +[      ]*[1-9][0-9]*[  ]+movslq        %al, %cx
> +[      ]*[1-9][0-9]*[  ]+movslq        %ax, %cx
> +[      ]*[1-9][0-9]*[  ]+movslq        %eax, %cx
> +[      ]*[1-9][0-9]*[  ]+movslq        %rax, %cx
> +[      ]*[1-9][0-9]*[  ]*
> +[      ]*[1-9][0-9]*[  ]+movslq        %al, %ecx
> +[      ]*[1-9][0-9]*[  ]+movslq        %ax, %ecx
> +[      ]*[1-9][0-9]*[  ]+movslq        %eax, %ecx
> +[      ]*[1-9][0-9]*[  ]+movslq        %rax, %ecx
> +[      ]*[1-9][0-9]*[  ]*
> +[      ]*[1-9][0-9]*[  ]+movslq        %al, %rcx
> +[      ]*[1-9][0-9]*[  ]+movslq        %ax, %rcx
> +[      ]*[1-9][0-9]* \?\?\?\? 4863C8[  ]+movslq        %eax, %rcx
> +[      ]*[1-9][0-9]*[  ]+movslq        %rax, %rcx
> +[      ]*[1-9][0-9]*[  ]*
>  [      ]*[1-9][0-9]*[  ]+movzx:
>  [      ]*[1-9][0-9]*[  ]+movzx %al, %cl
>  [      ]*[1-9][0-9]*[  ]+movzx %ax, %cl
> --- a/gas/testsuite/gas/i386/movx64.s
> +++ b/gas/testsuite/gas/i386/movx64.s
> @@ -241,6 +241,46 @@ movsx:
>         movswq  %eax, %rcx
>         movswq  %rax, %rcx
>
> +       movsl   %al, %cl
> +       movsl   %ax, %cl
> +       movsl   %eax, %cl
> +       movsl   %rax, %cl
> +
> +       movsl   %al, %cx
> +       movsl   %ax, %cx
> +       movsl   %eax, %cx
> +       movsl   %rax, %cx
> +
> +       movsl   %al, %ecx
> +       movsl   %ax, %ecx
> +       movsl   %eax, %ecx
> +       movsl   %rax, %ecx
> +
> +       movsl   %al, %rcx
> +       movsl   %ax, %rcx
> +       movsl   %eax, %rcx
> +       movsl   %rax, %rcx
> +
> +       movslq  %al, %cl
> +       movslq  %ax, %cl
> +       movslq  %eax, %cl
> +       movslq  %rax, %cl
> +
> +       movslq  %al, %cx
> +       movslq  %ax, %cx
> +       movslq  %eax, %cx
> +       movslq  %rax, %cx
> +
> +       movslq  %al, %ecx
> +       movslq  %ax, %ecx
> +       movslq  %eax, %ecx
> +       movslq  %rax, %ecx
> +
> +       movslq  %al, %rcx
> +       movslq  %ax, %rcx
> +       movslq  %eax, %rcx
> +       movslq  %rax, %rcx
> +
>  movzx:
>         movzx   %al, %cl
>         movzx   %ax, %cl
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -162,14 +162,9 @@ mov, 0xf24, None, Cpu386|CpuNo64, D|RegM
>  movbe, 0x0f38f0, None, CpuMovbe, D|Modrm|No_bSuf|No_sSuf|No_ldSuf, { Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
>
>  // Move with sign extend.
> -// "movsbl" & "movsbw" must not be unified into "movsb" to avoid
> -// conflict with the "movs" string move instruction.
> -movsbl, 0xfbe, None, Cpu386, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex, Reg32 }
> -movsbw, 0xfbe, None, Cpu386, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex, Reg16 }
> -movswl, 0xfbf, None, Cpu386, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Word|Unspecified|BaseIndex, Reg32 }
> -movsbq, 0xfbe, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Size64, { Reg8|Byte|Unspecified|BaseIndex, Reg64 }
> -movswq, 0xfbf, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Size64, { Reg16|Word|Unspecified|BaseIndex, Reg64 }
> -movslq, 0x63, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Size64, { Reg32|Dword|Unspecified|BaseIndex, Reg64 }
> +movsb, 0xfbe, None, Cpu386, Modrm|No_bSuf|No_sSuf|No_ldSuf|Pass2, { Reg8|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
> +movsw, 0xfbf, None, Cpu386, Modrm|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|Pass2, { Reg16|Unspecified|BaseIndex, Reg32|Reg64 }
> +movsl, 0x63, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|Pass2, { Reg32|Unspecified|BaseIndex, Reg64 }
>  movsx, 0xfbe, None, Cpu386, W|Modrm|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Reg16|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
>  movsx, 0x63, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg32|Reg64 }
>  movsxd, 0x63, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg32|Reg64 }
>
  
Jan Beulich Oct. 26, 2022, 9:07 a.m. UTC | #2
On 25.10.2022 19:10, H.J. Lu wrote:
> On Tue, Oct 25, 2022 at 12:29 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> PR gas/29524
>>
>> Having templates with a suffix explicitly present has always been
>> quirky. After prior adjustment all that's left to also eliminate the
> 
> I prefer keeping the current MOVS behavior.  Please submit a new patch
> set without MOVS changes.

You're kidding? If you _still_ think this patch isn't worth it despite
its merits, and despite it now only reducing the set of templates and
adjusting/extending the testsuite, without any changes to tc-i386.c you
can simply give your okay to the first 7 patches. Why would I spam the
list with an identical re-submission of those first 7 patches?

Independent of that I'd like to understand the reasons for your
preference, in particular when considering the benefits as well as the
reported bug which is being fixed (and which you marked as WONTFIX for
a subjective reason). So far your objection was that the change
complicates the code. Now it actually simplifies it slightly by
halving the number of involved templates.

Jan
  
H.J. Lu Oct. 27, 2022, 12:11 a.m. UTC | #3
On Wed, Oct 26, 2022 at 2:07 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 25.10.2022 19:10, H.J. Lu wrote:
> > On Tue, Oct 25, 2022 at 12:29 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> PR gas/29524
> >>
> >> Having templates with a suffix explicitly present has always been
> >> quirky. After prior adjustment all that's left to also eliminate the
> >
> > I prefer keeping the current MOVS behavior.  Please submit a new patch
> > set without MOVS changes.
>
> You're kidding? If you _still_ think this patch isn't worth it despite
> its merits, and despite it now only reducing the set of templates and
> adjusting/extending the testsuite, without any changes to tc-i386.c you

Without this change, the Pass2 change can be dropped.

> can simply give your okay to the first 7 patches. Why would I spam the
> list with an identical re-submission of those first 7 patches?
>
> Independent of that I'd like to understand the reasons for your
> preference, in particular when considering the benefits as well as the
> reported bug which is being fixed (and which you marked as WONTFIX for
> a subjective reason). So far your objection was that the change
> complicates the code. Now it actually simplifies it slightly by
> halving the number of involved templates.
>
> Jan
  
Jan Beulich Oct. 27, 2022, 6:31 a.m. UTC | #4
On 27.10.2022 02:11, H.J. Lu wrote:
> On Wed, Oct 26, 2022 at 2:07 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 25.10.2022 19:10, H.J. Lu wrote:
>>> On Tue, Oct 25, 2022 at 12:29 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> PR gas/29524
>>>>
>>>> Having templates with a suffix explicitly present has always been
>>>> quirky. After prior adjustment all that's left to also eliminate the
>>>
>>> I prefer keeping the current MOVS behavior.  Please submit a new patch
>>> set without MOVS changes.
>>
>> You're kidding? If you _still_ think this patch isn't worth it despite
>> its merits, and despite it now only reducing the set of templates and
>> adjusting/extending the testsuite, without any changes to tc-i386.c you
> 
> Without this change, the Pass2 change can be dropped.

No, it cannot.

Jan

>> can simply give your okay to the first 7 patches. Why would I spam the
>> list with an identical re-submission of those first 7 patches?
>>
>> Independent of that I'd like to understand the reasons for your
>> preference, in particular when considering the benefits as well as the
>> reported bug which is being fixed (and which you marked as WONTFIX for
>> a subjective reason). So far your objection was that the change
>> complicates the code. Now it actually simplifies it slightly by
>> halving the number of involved templates.
>>
>> Jan
> 
> 
>
  

Patch

--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -73,6 +73,7 @@  if [gas_32_check] then {
     run_dump_test "amd"
     run_dump_test "katmai"
     run_dump_test "jump"
+    run_dump_test "movs32"
     run_dump_test "movz32"
     run_dump_test "relax-1"
     run_dump_test "relax-2"
@@ -804,6 +805,7 @@  if [gas_64_check] then {
     run_dump_test "x86-64-segovr"
     run_list_test "x86-64-inval-seg" "-al"
     run_dump_test "x86-64-branch"
+    run_dump_test "movs64"
     run_dump_test "movz64"
     run_dump_test "x86-64-relax-1"
     run_dump_test "svme64"
--- /dev/null
+++ b/gas/testsuite/gas/i386/movs.s
@@ -0,0 +1,39 @@ 
+	.text
+movs:
+	movsb	%al,%ax
+	movsb	(%eax),%ax
+	movsb	%al,%eax
+	movsb	(%eax),%eax
+.ifdef x86_64
+	movsb	%al,%rax
+	movsb	(%rax),%rax
+.endif
+
+	movsbw	%al,%ax
+	movsbw	(%eax),%ax
+	movsbl	%al,%eax
+	movsbl	(%eax),%eax
+.ifdef x86_64
+	movsbq	%al,%rax
+	movsbq	(%rax),%rax
+.endif
+
+	movsw	%ax,%eax
+	movsw	(%eax),%eax
+.ifdef x86_64
+	movsw	%ax,%rax
+	movsw	(%rax),%rax
+.endif
+
+	movswl	%ax,%eax
+	movswl	(%eax),%eax
+.ifdef x86_64
+	movswq	%ax,%rax
+	movswq	(%rax),%rax
+
+	movsl	%eax,%rax
+	movsl	(%rax),%rax
+
+	movslq	%eax,%rax
+	movslq	(%rax),%rax
+.endif
--- /dev/null
+++ b/gas/testsuite/gas/i386/movs32.d
@@ -0,0 +1,22 @@ 
+#objdump: -dw
+#source: movs.s
+#name: x86 mov with sign-extend (32-bit object)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <movs>:
+[ 	]*[a-f0-9]+:	66 0f be c0 *	movsbw %al,%ax
+[ 	]*[a-f0-9]+:	66 0f be 00 *	movsbw \(%eax\),%ax
+[ 	]*[a-f0-9]+:	0f be c0 *	movsbl %al,%eax
+[ 	]*[a-f0-9]+:	0f be 00 *	movsbl \(%eax\),%eax
+[ 	]*[a-f0-9]+:	66 0f be c0 *	movsbw %al,%ax
+[ 	]*[a-f0-9]+:	66 0f be 00 *	movsbw \(%eax\),%ax
+[ 	]*[a-f0-9]+:	0f be c0 *	movsbl %al,%eax
+[ 	]*[a-f0-9]+:	0f be 00 *	movsbl \(%eax\),%eax
+[ 	]*[a-f0-9]+:	0f bf c0 *	movswl %ax,%eax
+[ 	]*[a-f0-9]+:	0f bf 00 *	movswl \(%eax\),%eax
+[ 	]*[a-f0-9]+:	0f bf c0 *	movswl %ax,%eax
+[ 	]*[a-f0-9]+:	0f bf 00 *	movswl \(%eax\),%eax
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/i386/movs64.d
@@ -0,0 +1,30 @@ 
+#objdump: -dw
+#source: movs.s
+#name: x86 mov with sign-extend (64-bit object)
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+ <movs>:
+[ 	]*[a-f0-9]+:	66 0f be c0 *	movsbw %al,%ax
+[ 	]*[a-f0-9]+:	67 66 0f be 00 *	movsbw \(%eax\),%ax
+[ 	]*[a-f0-9]+:	0f be c0 *	movsbl %al,%eax
+[ 	]*[a-f0-9]+:	67 0f be 00 *	movsbl \(%eax\),%eax
+[ 	]*[a-f0-9]+:	48 0f be c0 *	movsbq %al,%rax
+[ 	]*[a-f0-9]+:	48 0f be 00 *	movsbq \(%rax\),%rax
+[ 	]*[a-f0-9]+:	66 0f be c0 *	movsbw %al,%ax
+[ 	]*[a-f0-9]+:	67 66 0f be 00 *	movsbw \(%eax\),%ax
+[ 	]*[a-f0-9]+:	0f be c0 *	movsbl %al,%eax
+[ 	]*[a-f0-9]+:	67 0f be 00 *	movsbl \(%eax\),%eax
+[ 	]*[a-f0-9]+:	48 0f be c0 *	movsbq %al,%rax
+[ 	]*[a-f0-9]+:	48 0f be 00 *	movsbq \(%rax\),%rax
+[ 	]*[a-f0-9]+:	0f bf c0 *	movswl %ax,%eax
+[ 	]*[a-f0-9]+:	67 0f bf 00 *	movswl \(%eax\),%eax
+[ 	]*[a-f0-9]+:	48 0f bf c0 *	movswq %ax,%rax
+[ 	]*[a-f0-9]+:	48 0f bf 00 *	movswq \(%rax\),%rax
+[ 	]*[a-f0-9]+:	0f bf c0 *	movswl %ax,%eax
+[ 	]*[a-f0-9]+:	67 0f bf 00 *	movswl \(%eax\),%eax
+[ 	]*[a-f0-9]+:	48 0f bf c0 *	movswq %ax,%rax
+[ 	]*[a-f0-9]+:	48 0f bf 00 *	movswq \(%rax\),%rax
+#pass
--- a/gas/testsuite/gas/i386/movx16.l
+++ b/gas/testsuite/gas/i386/movx16.l
@@ -41,11 +41,11 @@ 
 [ 	]*[1-9][0-9]*[ 	]+movsb	%ax, %cl
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %cl
 [ 	]*[1-9][0-9]*[ 	]*
-[ 	]*[1-9][0-9]*[ 	]+movsb	%al, %cx
+[ 	]*[1-9][0-9]* \?\?\?\? 0FBEC8[ 	]+movsb	%al, %cx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%ax, %cx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %cx
 [ 	]*[1-9][0-9]*[ 	]*
-[ 	]*[1-9][0-9]*[ 	]+movsb	%al, %ecx
+[ 	]*[1-9][0-9]* \?\?\?\? 660FBEC8[ 	]+movsb	%al, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%ax, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %ecx
 [ 	]*[1-9][0-9]*[ 	]*
@@ -82,7 +82,7 @@ 
 [ 	]*[1-9][0-9]*[ 	]+movsw	%eax, %cx
 [ 	]*[1-9][0-9]*[ 	]*
 [ 	]*[1-9][0-9]*[ 	]+movsw	%al, %ecx
-[ 	]*[1-9][0-9]*[ 	]+movsw	%ax, %ecx
+[ 	]*[1-9][0-9]* \?\?\?\? 660FBFC8[ 	]+movsw	%ax, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsw	%eax, %ecx
 [ 	]*[1-9][0-9]*[ 	]*
 [ 	]*[1-9][0-9]*[ 	]+movswl	%al, %cl
--- a/gas/testsuite/gas/i386/movx32.l
+++ b/gas/testsuite/gas/i386/movx32.l
@@ -41,11 +41,11 @@ 
 [ 	]*[1-9][0-9]*[ 	]+movsb	%ax, %cl
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %cl
 [ 	]*[1-9][0-9]*[ 	]*
-[ 	]*[1-9][0-9]*[ 	]+movsb	%al, %cx
+[ 	]*[1-9][0-9]* \?\?\?\? 660FBEC8[ 	]+movsb	%al, %cx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%ax, %cx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %cx
 [ 	]*[1-9][0-9]*[ 	]*
-[ 	]*[1-9][0-9]*[ 	]+movsb	%al, %ecx
+[ 	]*[1-9][0-9]* \?\?\?\? 0FBEC8[ 	]+movsb	%al, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%ax, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %ecx
 [ 	]*[1-9][0-9]*[ 	]*
@@ -82,7 +82,7 @@ 
 [ 	]*[1-9][0-9]*[ 	]+movsw	%eax, %cx
 [ 	]*[1-9][0-9]*[ 	]*
 [ 	]*[1-9][0-9]*[ 	]+movsw	%al, %ecx
-[ 	]*[1-9][0-9]*[ 	]+movsw	%ax, %ecx
+[ 	]*[1-9][0-9]* \?\?\?\? 0FBFC8[ 	]+movsw	%ax, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsw	%eax, %ecx
 [ 	]*[1-9][0-9]*[ 	]*
 [ 	]*[1-9][0-9]*[ 	]+movswl	%al, %cl
--- a/gas/testsuite/gas/i386/movx64.l
+++ b/gas/testsuite/gas/i386/movx64.l
@@ -106,17 +106,17 @@ 
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %cl
 [ 	]*[1-9][0-9]*[ 	]+movsb	%rax, %cl
 [ 	]*[1-9][0-9]*[ 	]*
-[ 	]*[1-9][0-9]*[ 	]+movsb	%al, %cx
+[ 	]*[1-9][0-9]* \?\?\?\? 660FBEC8[ 	]+movsb	%al, %cx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%ax, %cx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %cx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%rax, %cx
 [ 	]*[1-9][0-9]*[ 	]*
-[ 	]*[1-9][0-9]*[ 	]+movsb	%al, %ecx
+[ 	]*[1-9][0-9]* \?\?\?\? 0FBEC8[ 	]+movsb	%al, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%ax, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%rax, %ecx
 [ 	]*[1-9][0-9]*[ 	]*
-[ 	]*[1-9][0-9]*[ 	]+movsb	%al, %rcx
+[ 	]*[1-9][0-9]* \?\?\?\? 480FBEC8[ 	]+movsb	%al, %rcx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%ax, %rcx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%eax, %rcx
 [ 	]*[1-9][0-9]*[ 	]+movsb	%rax, %rcx
@@ -192,12 +192,12 @@ 
 [ 	]*[1-9][0-9]*[ 	]+movsw	%rax, %cx
 [ 	]*[1-9][0-9]*[ 	]*
 [ 	]*[1-9][0-9]*[ 	]+movsw	%al, %ecx
-[ 	]*[1-9][0-9]*[ 	]+movsw	%ax, %ecx
+[ 	]*[1-9][0-9]* \?\?\?\? 0FBFC8[ 	]+movsw	%ax, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsw	%eax, %ecx
 [ 	]*[1-9][0-9]*[ 	]+movsw	%rax, %ecx
 [ 	]*[1-9][0-9]*[ 	]*
 [ 	]*[1-9][0-9]*[ 	]+movsw	%al, %rcx
-[ 	]*[1-9][0-9]*[ 	]+movsw	%ax, %rcx
+[ 	]*[1-9][0-9]* \?\?\?\? 480FBFC8[ 	]+movsw	%ax, %rcx
 [ 	]*[1-9][0-9]*[ 	]+movsw	%eax, %rcx
 [ 	]*[1-9][0-9]*[ 	]+movsw	%rax, %rcx
 [ 	]*[1-9][0-9]*[ 	]*
@@ -241,6 +241,46 @@ 
 [ 	]*[1-9][0-9]*[ 	]+movswq	%eax, %rcx
 [ 	]*[1-9][0-9]*[ 	]+movswq	%rax, %rcx
 [ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+movsl	%al, %cl
+[ 	]*[1-9][0-9]*[ 	]+movsl	%ax, %cl
+[ 	]*[1-9][0-9]*[ 	]+movsl	%eax, %cl
+[ 	]*[1-9][0-9]*[ 	]+movsl	%rax, %cl
+[ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+movsl	%al, %cx
+[ 	]*[1-9][0-9]*[ 	]+movsl	%ax, %cx
+[ 	]*[1-9][0-9]*[ 	]+movsl	%eax, %cx
+[ 	]*[1-9][0-9]*[ 	]+movsl	%rax, %cx
+[ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+movsl	%al, %ecx
+[ 	]*[1-9][0-9]*[ 	]+movsl	%ax, %ecx
+[ 	]*[1-9][0-9]*[ 	]+movsl	%eax, %ecx
+[ 	]*[1-9][0-9]*[ 	]+movsl	%rax, %ecx
+[ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+movsl	%al, %rcx
+[ 	]*[1-9][0-9]*[ 	]+movsl	%ax, %rcx
+[ 	]*[1-9][0-9]* \?\?\?\? 4863C8[ 	]+movsl	%eax, %rcx
+[ 	]*[1-9][0-9]*[ 	]+movsl	%rax, %rcx
+[ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+movslq	%al, %cl
+[ 	]*[1-9][0-9]*[ 	]+movslq	%ax, %cl
+[ 	]*[1-9][0-9]*[ 	]+movslq	%eax, %cl
+[ 	]*[1-9][0-9]*[ 	]+movslq	%rax, %cl
+[ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+movslq	%al, %cx
+[ 	]*[1-9][0-9]*[ 	]+movslq	%ax, %cx
+[ 	]*[1-9][0-9]*[ 	]+movslq	%eax, %cx
+[ 	]*[1-9][0-9]*[ 	]+movslq	%rax, %cx
+[ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+movslq	%al, %ecx
+[ 	]*[1-9][0-9]*[ 	]+movslq	%ax, %ecx
+[ 	]*[1-9][0-9]*[ 	]+movslq	%eax, %ecx
+[ 	]*[1-9][0-9]*[ 	]+movslq	%rax, %ecx
+[ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+movslq	%al, %rcx
+[ 	]*[1-9][0-9]*[ 	]+movslq	%ax, %rcx
+[ 	]*[1-9][0-9]* \?\?\?\? 4863C8[ 	]+movslq	%eax, %rcx
+[ 	]*[1-9][0-9]*[ 	]+movslq	%rax, %rcx
+[ 	]*[1-9][0-9]*[ 	]*
 [ 	]*[1-9][0-9]*[ 	]+movzx:
 [ 	]*[1-9][0-9]*[ 	]+movzx	%al, %cl
 [ 	]*[1-9][0-9]*[ 	]+movzx	%ax, %cl
--- a/gas/testsuite/gas/i386/movx64.s
+++ b/gas/testsuite/gas/i386/movx64.s
@@ -241,6 +241,46 @@  movsx:
 	movswq	%eax, %rcx
 	movswq	%rax, %rcx
 
+	movsl	%al, %cl
+	movsl	%ax, %cl
+	movsl	%eax, %cl
+	movsl	%rax, %cl
+
+	movsl	%al, %cx
+	movsl	%ax, %cx
+	movsl	%eax, %cx
+	movsl	%rax, %cx
+
+	movsl	%al, %ecx
+	movsl	%ax, %ecx
+	movsl	%eax, %ecx
+	movsl	%rax, %ecx
+
+	movsl	%al, %rcx
+	movsl	%ax, %rcx
+	movsl	%eax, %rcx
+	movsl	%rax, %rcx
+
+	movslq	%al, %cl
+	movslq	%ax, %cl
+	movslq	%eax, %cl
+	movslq	%rax, %cl
+
+	movslq	%al, %cx
+	movslq	%ax, %cx
+	movslq	%eax, %cx
+	movslq	%rax, %cx
+
+	movslq	%al, %ecx
+	movslq	%ax, %ecx
+	movslq	%eax, %ecx
+	movslq	%rax, %ecx
+
+	movslq	%al, %rcx
+	movslq	%ax, %rcx
+	movslq	%eax, %rcx
+	movslq	%rax, %rcx
+
 movzx:
 	movzx	%al, %cl
 	movzx	%ax, %cl
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -162,14 +162,9 @@  mov, 0xf24, None, Cpu386|CpuNo64, D|RegM
 movbe, 0x0f38f0, None, CpuMovbe, D|Modrm|No_bSuf|No_sSuf|No_ldSuf, { Word|Dword|Qword|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 
 // Move with sign extend.
-// "movsbl" & "movsbw" must not be unified into "movsb" to avoid
-// conflict with the "movs" string move instruction.
-movsbl, 0xfbe, None, Cpu386, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex, Reg32 }
-movsbw, 0xfbe, None, Cpu386, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex, Reg16 }
-movswl, 0xfbf, None, Cpu386, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg16|Word|Unspecified|BaseIndex, Reg32 }
-movsbq, 0xfbe, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Size64, { Reg8|Byte|Unspecified|BaseIndex, Reg64 }
-movswq, 0xfbf, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Size64, { Reg16|Word|Unspecified|BaseIndex, Reg64 }
-movslq, 0x63, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Size64, { Reg32|Dword|Unspecified|BaseIndex, Reg64 }
+movsb, 0xfbe, None, Cpu386, Modrm|No_bSuf|No_sSuf|No_ldSuf|Pass2, { Reg8|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
+movsw, 0xfbf, None, Cpu386, Modrm|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|Pass2, { Reg16|Unspecified|BaseIndex, Reg32|Reg64 }
+movsl, 0x63, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|Pass2, { Reg32|Unspecified|BaseIndex, Reg64 }
 movsx, 0xfbe, None, Cpu386, W|Modrm|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Reg16|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
 movsx, 0x63, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg32|Reg64 }
 movsxd, 0x63, None, Cpu64, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Unspecified|BaseIndex, Reg32|Reg64 }