[PATCH 02/12] x86: fold various AVX512VL templates into their AVX512F counterparts

[H.J. Lu]

On Sun, Jun 3, 2018 at 11:55 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 01.06.18 at 18:28, <hjl.tools@gmail.com> wrote:
>> On Thu, May 31, 2018 at 11:30 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 31.05.18 at 15:36, <hjl.tools@gmail.com> wrote:
>>>> On Wed, May 30, 2018 at 9:11 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Wed, May 30, 2018 at 7:46 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> --- a/opcodes/i386-opc.tbl
>>>>>> +++ b/opcodes/i386-opc.tbl
>>>>>> @@ -2914,22 +2914,22 @@ kshiftrw, 3, 0x6630, None, 1, CpuAVX512F
>>>>>>
>>>>>>  kunpckbw, 3, 0x664B, None, 1, CpuAVX512F, Modrm|Vex=2|VexOpcode=0|VexVVVV=1|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegMask, RegMask, RegMask }
>>>>>>
>>>>>> -vaddpd, 3, 0x6658, None, 1, CpuAVX512F, Modrm|EVex=1|Masking=3|VexOpcode=0|VexVVVV=1|VexW=2|Broadcast|Disp8MemShift=6|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegZMM|Qword|ZMMword|Unspecified|BaseIndex, RegZMM, RegZMM }
>>>>>
>>>>> EVex=1 is removed.  But it isn't mentioned.
>>>
>>> I don't understand this remark: This is a natural effect of folding, as
>>> already done in earlier commits (see e.g. e2195274d4 where this isn't
>>> mentioned explicitly either). If we want to express multiple operand
>>> sizes with a single template, we can't specify Evex= , except (where
>>> is_vex_encoding() can't otherwise be made return true) perhaps
>>> EVex=5.
>>>
>>>>>> +vaddpd, 3, 0x6658, None, 1, CpuAVX512F, Modrm|Masking=3|VexOpcode=0|VexVVVV=1|VexW=2|Broadcast|Disp8ShiftVL|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegXMM|RegYMM|RegZMM|Qword|Unspecified|BaseIndex, RegXMM|RegYMM|RegZMM, RegXMM|RegYMM|RegZMM }
>>>>>>
>>>>>
>>>>> Where is Disp8ShiftVL defined?
>>>
>>> In i386-gen.c. The ChangeLog entry doesn't contain this new
>>> shorthand because the array containing it is new, and hence its
>>> name is what is mentioned instead.
>>>
>>>> Please resend the complete set of patches.
>>>
>>> I don't understand why I would need to. All I've omitted are the
>>> generated files, as usual.
>>>
>>
>> Please update your commit log to describe what your patch does
>> to i386-opc.tbl.
>
> Would
>
>         * i386-opc.tbl: Fold AVX512VL templates into their respective
>         AVX512F counterparts where possible, using Disp8ShiftVL instead
>         of Evex= plus Disp8MemShift= as appropriate.
>
> suffice, or what level of detail do you expect? Enumeration of all insns?
> What good would going through such a tedious exercise do? Please also
> look back at the quoted commit, where you had approved the patch with
> a brief ChangeLog entry just like the original one here.

what is the advantage of

+static const initializer opcode_modifier_shorthands[] =
+{
+  { "Disp8ShiftVL", "Disp8MemShift=" stringify(DISP8_SHIFT_VL) },
+};
+

How much does it save?  I can't tell what an entry has by just
looking at i386-opc.h.

> In any event, patches 3...6 would be similarly updated then, unless you
> tell me that the more brief variant is fine there.
>
> Jan
>
>



-- 
H.J.