This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] x86: Determine vector length from the last vector operand
>>> On 24.07.18 at 15:43, <hjl.tools@gmail.com> wrote:
> On Tue, Jul 24, 2018 at 5:34 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 24.07.18 at 14:08, <hjl.tools@gmail.com> wrote:
>>> On Tue, Jul 24, 2018 at 4:55 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 24.07.18 at 13:38, <hjl.tools@gmail.com> wrote:
>>>>> On Tue, Jul 24, 2018 at 12:41 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>>> On 22.07.18 at 21:02, <hjl.tools@gmail.com> wrote:
>>>>>>> Determine VEX/EVEXE vector length from the last multi-length vector
>>>>>>> operand.
>>>>>>
>>>>>> I see you've committed this already: It would have been really
>>>>>> helpful to say _why_ you're doing the change in the commit message.
>>>>>> For posterity as well as my understanding - could you at least do so
>>>>>> here please? That's even more so that VEX and EVEX processing
>>>>>> differed in that regard before your change.
>>>>>
>>>>> The encoding can be determined by the last multi-length vector
>>>>> operand. There is no need to scan from the start.
>>>>
>>>> But why is scanning from the rear better? Immediates usually sit at the
>>>> end. In the EVEX case, considering the break-s you add, this may
>>>> indeed be more efficient, but the same then wouldn't hold for the VEX
>>>
>>> break can be used only when scanning from the last operand.
>>
>> Yes - that's why I understand (even without the patch description
>> saying so) why this way it's more efficient in the EVEX case. The
>> VEX case, otoh, is still unclear to me.
>>
>>>>>>> --- a/gas/config/tc-i386.c
>>>>>>> +++ b/gas/config/tc-i386.c
>>>>>>> @@ -3363,10 +3363,12 @@ build_vex_prefix (const insn_template *t)
>>>>>>> vector_length = 1;
>>>>>>> else
>>>>>>> {
>>>>>>> - unsigned int op;
>>>>>>> + int op;
>>>>>>
>>>>>> This is the sort of change I would have objected to, btw. Variables
>>>>>> used to index arrays should be unsigned whenever possible. And
>>>>>> doing so would have been easy enough here:
>>>>>
>>>>> I want
>>>>>
>>>>> if (op < 0)
>>>>> abort ();
>>>>>
>>>>> after the loop.
>>>>
>>>> if (op >= MAX_OPERANDS)
>>>> abort ()
>>>>
>>>> would be equivalent afaict.
>>>
>>> It won't work when scanning from the last operand.
>>
>> Why would it not? The value will wrap through zero to UINT_MAX.
>
> I don't see there are significant differences here.
> It is perfectly OK to use "int" as index.
Generated code is worse on many architectures: Often sub-machine-
word size loads (and on x86-64 also other operations) are zero-
extending. A signed index hence needs sign-extending in a separate
step, rather than being able to directly use the result of the load (or
other operation).
>>>>>>> if (i.tm.operand_types[op].bitfield.xmmword
>>>>>>> + i.tm.operand_types[op].bitfield.ymmword
>>>>>>> + i.tm.operand_types[op].bitfield.zmmword > 1)
>>>>>>> {
>>>>>>> if (i.types[op].bitfield.zmmword)
>>>>>>> - i.tm.opcode_modifier.evex = EVEX512;
>>>>>>> + {
>>>>>>> + i.tm.opcode_modifier.evex = EVEX512;
>>>>>>> + break;
>>>>>>> + }
>>>>>>> else if (i.types[op].bitfield.ymmword)
>>>>>>> - i.tm.opcode_modifier.evex = EVEX256;
>>>>>>> + {
>>>>>>> + i.tm.opcode_modifier.evex = EVEX256;
>>>>>>> + break;
>>>>>>> + }
>>>>>>> else if (i.types[op].bitfield.xmmword)
>>>>>>> - i.tm.opcode_modifier.evex = EVEX128;
>>>>>>> + {
>>>>>>> + i.tm.opcode_modifier.evex = EVEX128;
>>>>>>> + break;
>>>>>>> + }
>>>>>>> else if (i.broadcast && (int) op == i.broadcast->operand)
>>>>>>
>>>>>> This has rendered all the "else" pointless.
>>>>>
>>>>> What do you mean? Isn't it used in
>>>>>
>>>>> vcvttpd2uqq, 2, 0x6678, None, 1, CpuAVX512DQ,
>>>>> Modrm|Masking=3|VexOpcode=0|VexW=2|Broadcast|Disp8ShiftVL|CheckRegSize|No_bS
>>>>> uf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf,
>>>>> { RegXMM|RegYMM|RegZMM|Qword|Unspecified|BaseIndex,
>>>>> RegXMM|RegYMM|RegZMM }
>>>>
>>>> I didn't mean to drop the "else if (...)", just the "else" (with the
>>>> if () to stay).
>>>>
>>>
>>> Scanning from the last operand is more efficient for both EVEX and VEX.
>>
>> Well, as per above - I can see why in the EVEX case, but not (yet) in
>> the VEX one. But I also can't associate your response here to the
>> point made in immediate context.
>
> For
>
> vgf2p8mulb, 3, 0x66cf, None, 1, CpuAVX|CpuGFNI,
> Modrm|Vex|VexOpcode=1|VexVVVV=1|VexW=1|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|
> No_sSuf|No_qSuf|No_ldSuf,
> { Unspecified|BaseIndex|RegXMM|RegYMM, RegXMM|RegYMM, RegXMM|RegYMM }
>
> we only need to check the last operand, instead of the first and
> second operands.
>
> vgf2p8mulb (%ecx),%ymm4,%ymm6
Okay, if that's a commonly applicable pattern, fine.
Jan