Re: [PATCH 1/6] Enable Intel AVX512_VBMI2 instructions

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Nov 21, 2017 at 10:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Nov 21, 2017 at 7:19 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 21.11.17 at 16:01, <igor.v.tsimbalist@intel.com> wrote:
>>>>  -----Original Message-----
>>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>>> Sent: Monday, November 20, 2017 9:07 AM
>>>> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
>>>> Cc: Lu, Hongjiu <hongjiu.lu@intel.com>; binutils@sourceware.org
>>>> Subject: RE: [PATCH 1/6] Enable Intel AVX512_VBMI2 instructions
>>>>
>>>> >>> On 21.10.17 at 11:15, <igor.v.tsimbalist@intel.com> wrote:
>>>> > Resending the patch after regenerated files removal  (mailer-daemon
>>>> > complained about the size of the patch).
>>>>
>>>> What's the point of the Vec_Disp8 ...
>>>>
>>>> >+vpcompressb, 2, 0x6663, None, 1, CpuAVX512_VBMI2,
>>>> Modrm|EVex=1|Masking=2|VexOpcode=1|VexW=1|IgnoreSize|No_bSuf|
>>>> No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegZMM,
>>>> ZMMword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S|Vec_Disp
>>>> 8 }
>>>> >+vpcompressb, 2, 0x6663, None, 1, CpuAVX512_VBMI2|CpuAVX512VL,
>>>> Modrm|EVex=2|Masking=2|VexOpcode=1|VexW=1|IgnoreSize|No_bSuf|
>>>> No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegXMM,
>>>> XMMword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S|Vec_Disp
>>>> 8 }
>>>> >+vpcompressb, 2, 0x6663, None, 1, CpuAVX512_VBMI2|CpuAVX512VL,
>>>> Modrm|EVex=3|Masking=2|VexOpcode=1|VexW=1|IgnoreSize|No_bSuf|
>>>> No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegYMM,
>>>> YMMword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S|Vec_Disp
>>>> 8 }
>>>>
>>>> ... here and ...
>>>>
>>>> >+vpexpandb, 2, 0x6662, None, 1, CpuAVX512_VBMI2,
>>>> Modrm|EVex=1|Masking=3|VexOpcode=1|VexW=1|IgnoreSize|No_bSuf|
>>>> No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, {
>>>> RegZMM|ZMMword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S
>>>> |Vec_Disp8, RegZMM }
>>>> >+vpexpandb, 2, 0x6662, None, 1, CpuAVX512_VBMI2|CpuAVX512VL,
>>>> Modrm|EVex=2|Masking=3|VexOpcode=1|VexW=1|IgnoreSize|No_bSuf|
>>>> No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, {
>>>> RegXMM|XMMword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S
>>>> |Vec_Disp8, RegXMM }
>>>> >+vpexpandb, 2, 0x6662, None, 1, CpuAVX512_VBMI2|CpuAVX512VL,
>>>> Modrm|EVex=3|Masking=3|VexOpcode=1|VexW=1|IgnoreSize|No_bSuf|
>>>> No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, {
>>>> RegYMM|YMMword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S
>>>> |Vec_Disp8, RegYMM }
>>>>
>>>> ? There's no Disp8 scaling here. Vec_Disp8 really is mostly (and
>>>> perhaps entirely, i.e. I'm in the process of determining whether we
>>>> can get rid of it) redundant with Disp8MemShift beng non-zero.
>>>
>>> Vec_Disp8 is redundant here, agree.
>>>
>>>> Also what was the point of inserting a stray blank line at the top
>>>> of opcodes/i386-opc.tbl?
>>>
>>> It's obviously an accidental change. Thanks for pointing this out.
>>>
>>> Attached is a patch to fix this. Ok for trunk?
>>
>> That's H.J.'s decision, but afaic I'd prefer the changelog entry
>> for i386-opc.tbl to be a little more specific - you're not globally
>> removing Vec_Disp8 here, that's only something that should be
>> done eventually.
>>
>
> Please change it to
>
> * opcodes/i386-opc.tbl: Remove Vec_Disp8 from vpcompressb and
^^^^^^^^^^^^^  Please drop "opcodes/".

> vpexpandb.
>
> OK with this change.
>


-- 
H.J.