[PATCH] x86: Determine vector length from the last vector operand

[H.J. Lu]

On Tue, Jul 24, 2018 at 6:57 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jul 24, 2018 at 6:50 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> 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).
>>
>
> Sure.  Go ahead if you want to change it to unsigned.
>

This is what I checked in.


-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Use-unsigned-int-to-iterate-through-vector-opera.patch
Type: text/x-patch
Size: 2278 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20180724/0c9447f0/attachment.bin>