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

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

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.

-- 
H.J.