Re: [PATCH] x86: Print {bad} on invalid broadcast in OP_E_memory

[Jan Beulich <jbeulich@suse.com>]

On 27.09.2021 11:39, Cui,Lili wrote:
> Hi,
> 
> This patch is to fix PR 28381,
> 
> Make check-gas is ok.
> 
> Ok for master ?
> 
> 
> Don't print broadcast for scalar_mode, and print {bad} for invalid broadcast.
> 
> gas/
> 
> 	PR binutils/28381
> 	* testsuite/gas/i386/bad-bcast.s: Add a new testcase.
> 	* testsuite/gas/i386/bad-bcast.d: Likewise.
> 
> opcodes/
> 
> 	PR binutils/28381
> 	* i386-dis.c (static struct): Add no_broadcast.
> 	(OP_E_memory): Mark invalid broadcast with no_broadcast=1 and Print "{bad}"for it.
> 	(intel_operand_size): mark invalid broadcast with no_broadcast=1.
> 	(OP_XMM): Mark scalar_mode with no_broadcast=1.
> ---
>  gas/testsuite/gas/i386/bad-bcast.d |  10 +-
>  gas/testsuite/gas/i386/bad-bcast.s |   2 +
>  opcodes/i386-dis.c                 | 155 +++++++++++++++--------------
>  3 files changed, 88 insertions(+), 79 deletions(-)
> 
> diff --git a/gas/testsuite/gas/i386/bad-bcast.d b/gas/testsuite/gas/i386/bad-bcast.d
> index 9fc474a42f..2ebb16800a 100644
> --- a/gas/testsuite/gas/i386/bad-bcast.d
> +++ b/gas/testsuite/gas/i386/bad-bcast.d
> @@ -7,8 +7,8 @@
>  Disassembly of section .text:
>  
>  0+ <.text>:
> - +[a-f0-9]+:	62                   	.byte 0x62
> - +[a-f0-9]+:	c3                   	ret    
> - +[a-f0-9]+:	8c 1d 66 90 66 90    	mov    %ds,0x90669066
> - +[a-f0-9]+:	66 90                	xchg   %ax,%ax
> -#pass
> + +[a-f0-9]+:	62 c3 8c 1d 66\s+\(bad\)
> + +[a-f0-9]+:	90\s+nop
> + +[a-f0-9]+:	66 90\s+xchg   %ax,%ax
> + +[a-f0-9]+:	66 90\s+xchg   %ax,%ax
> + +[a-f0-9]+:	62 c1 ff 3f 2a 20\s+vcvtsi2sd \(%eax\){bad},%xmm0,%xmm4{%k7}

I've indicated so in the past already (perhaps not to you, but in
general): I consider it wrong to add test cases with bogus expectations.
In the case here it's not only the broadcast that's invalid, but also
the {%k7}. For your purpose you don't need the masking part, so I'd like
to ask that you drop it. If and when we properly mark such bad uses of
masking, a separate test case covering that aspect will want adding, but
the one you add now would then better not require adjusting.

Jan