This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH 3/6] Enable Intel VAES instructions


>>> On 23.11.17 at 15:50, <igor.v.tsimbalist@intel.com> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Monday, November 20, 2017 9:14 AM
>> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
>> Cc: Lu, Hongjiu <hongjiu.lu@intel.com>; binutils@sourceware.org 
>> Subject: Re: [PATCH 3/6] Enable Intel VAES instructions
>> 
>> >>> On 21.10.17 at 11:19, <igor.v.tsimbalist@intel.com> wrote:
>> >--- /dev/null
>> >+++ b/gas/testsuite/gas/i386/avx512f_vaes-intel.d
>> >@@ -0,0 +1,36 @@
>> >+#as:
>> >+#objdump: -dw -Mintel
>> >+#name: i386 AVX512F/VAES insns (Intel disassembly)
>> >+#source: avx512f_vaes.s
>> >+
>> >+.*: +file format .*
>> >+
>> >+
>> >+Disassembly of section \.text:
>> >+
>> >+00000000 <_start>:
>> >+[ 	]*[a-f0-9]+:[ 	]*62 f2 55 48 de f4[ 	]*vaesdec
>> zmm6,zmm5,zmm4
>> >+[ 	]*[a-f0-9]+:[ 	]*62 f2 55 48 de b4 f4 c0 1d fe ff[ 	]*vaesdec
>> zmm6,zmm5,ZMMWORD PTR \[esp\+esi\*8-0x1e240\]
>> >+[ 	]*[a-f0-9]+:[ 	]*62 f2 55 48 de b2 c0 1f 00 00[ 	]*vaesdec
>> zmm6,zmm5,ZMMWORD PTR \[edx\+0x1fc0\]
>> 
>> Either the Disp32 encodings here and elsewhere are wrong (and
>> the opcode table entries lack Disp8MemShift attributes) or ...
>> 
>> >--- /dev/null
>> >+++ b/gas/testsuite/gas/i386/avx512f_vaes-wig.s
>> >@@ -0,0 +1,37 @@
>> >+# Check 32bit AVX512F,VAES WIG instructions
>> >+
>> >+	.allow_index_reg
>> >+	.text
>> >+_start:
>> >+	vaesdec	%zmm4, %zmm5, %zmm6	 # AVX512F,VAES
>> >+	vaesdec	-123456(%esp,%esi,8), %zmm5, %zmm6	 #
>> AVX512F,VAES
>> >+	vaesdec	8128(%edx), %zmm5, %zmm6	 # AVX512F,VAES
>> Disp8
>> 
>> ... Disp8 comments like this one are wrong. I've again noticed
>> this in the context of verifying the "Disp8MemShift != 0 if and
>> only if Vec_Disp8 set for the memory operand" equivalence.
> 
> There was a real bug, thank you for catching it!
> 
> The patch fixing this is attached. Ok for trunk?

You've got H.J.'s approval already, but I have to admit that I find
"Regenerate." for testsuite .d files irritating: It looks to me as if
you indeed more or less blindly take what the disassembler
produces for the given input. But that's what I assume has lead to
the bug going unnoticed which you've now fixed.

Jan


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]