Re: [PATCH 2/5] x86: improve SIMD to‑scalar‑int conversion insn handling

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

On Thu, Mar 22, 2018 at 6:40 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 22.03.18 at 13:46, <hjl.tools@gmail.com> wrote:
>> On Thu, Mar 22, 2018 at 5:42 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 22.03.18 at 13:18, <hjl.tools@gmail.com> wrote:
>>>> On Thu, Mar 22, 2018 at 5:06 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> On 22.03.18 at 12:54, <hjl.tools@gmail.com> wrote:
>>>>>> On Thu, Mar 22, 2018 at 4:42 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>>>> On 22.03.18 at 12:23, <hjl.tools@gmail.com> wrote:
>>>>>>>> On Thu, Mar 22, 2018 at 12:42 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>>>>>> On 21.03.18 at 20:17, <hjl.tools@gmail.com> wrote:
>>>>>>>>>> On Wed, Mar 21, 2018 at 7:20 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>>>>> In the course of folding their patterns (possible now that the pointless
>>>>>>>>>>> and partly even bogus VecESize are no longer in the way) I've noticed
>>>>>>>>>>> that vcvt*2usi, other than their vcvt*2si counterparts, didn't allow for
>>>>>>>>>>> any suffixes. As with all insns touching GPRs, these should be permitted
>>>>>>>>>>> even if they're not required for determining operand sizes. In turn I've
>>>>>>>>>>> noticed that only a very limited set of cases had a suffix added in
>>>>>>>>>>> disassembly with -Msuffix, while all suffixes should be output in that
>>>>>>>>>>> mode.
>>>>>>>>>>>
>>>>>>>>>>> gas/
>>>>>>>>>>> 2018-03-21  Jan Beulich  <jbeulich@suse.com>
>>>>>>>>>>>
>>>>>>>>>>>         * testsuite/gas/i386/cvt-2si.d, testsuite/gas/i386/cvt-2si.s:
>>>>>>>>>>>         New.
>>>>>>>>>>>         * testsuite/gas/i386/i386.exp: Run new test.
>>>>>>>>>>>         * testsuite/gas/i386/ilp32/x86-64-simd-suffix.d,
>>>>>>>>>>>         testsuite/gas/i386/simd-suffix.d,
>>>>>>>>>>>         testsuite/gas/i386/x86-64-simd-suffix.d: Adjust expectations.
>>>>>>>>>>>
>>>>>>>>>>> opcodes/
>>>>>>>>>>> 2018-03-21  Jan Beulich  <jbeulich@suse.com>
>>>>>>>>>>>
>>>>>>>>>>>         * i386-dis.c (prefix_table): Replace Y by S for cvt*2si.
>>>>>>>>>>>         (vex_len_table): Replace Y by S for vcvt*2si.
>>>>>>>>>>>         (putop): Replace plain 'Y' handling by abort().
>>>>>>>>>>>         * i386-dis-evex.h (evex_table): Replace Y by S for vcvt*2si.
>>>>>>>>>>>         * i386-opc.tbl (vcvt*d2si): Fold AVX512 forms. Add ToDword.
>>>>>>>>>>>         (vcvt*s2si): Fold AVX512 forms. Add ToQword.
>>>>>>>>>>>         * i386-tlb.h: Re-generate.
>>>>>>>>>>
>>>>>>>>>> I prefer not to add suffixes to vector instructions with GPRs unless it
>>>>>>>>>> is required.
>>>>>>>>>
>>>>>>>>> I'm afraid I don't follow - suffixes (in particular in suffix-always
>>>>>>>>> mode) aren't an optional thing. I actually consider it a mistake
>>>>>>>>> for the compiler to omit them, and the compiler _has to_ omit
>>>>>>>>> them right now because we don't accept them. Furthermore -
>>>>>>>>> did you look at the state things are currently in? If you didn't
>>>>>>>>> want suffixes when not needed, why is there the Y format in
>>>>>>>>> the first place? And why said inconsistency between 2usi and
>>>>>>>>> 2si conversions in the assembler? And more fundamentally -
>>>>>>>>> why are vector insns different from others touching GPRs?
>>>>[...]
>>>> If we exclude integer instructions from this discussion, vector
>>>> instructions are quite consistent without suffix.   Yes, some vector
>>>> instructions do need suffixes, but only a few.
>>>
>>> Some _need_ suffixes, yes. Various other _allow_ for suffixes.
>>> Once again, would you mind actually reading what I've said in the
>>> patch description as well as answering the questions I've raised?
>>> _I did point out_ the inconsistencies I'm seeing, yet you continue
>>> to talk in general terms.
>>
>> My point is that we shouldn't add suffix to vector instructions if
>> they can't be encoded uniquely.  That is OK If there are some
>> inconsistencies.
>
> Sigh. Why is the disassembler then adding a q suffix _sometimes_
> with -Msuffix? Just look at the patch _context_ of the changes to

I am OK to remove suffixes for -Msuffix on vector instructions.

> x86-64-simd-suffix.d to see what I'm talking about. Again, I did
> ask the questions about the purpose of Y as well as its inconsistent

Remove 'Y'.

> use before. Plus you should have objected to the AVX512 insns
> introducing unnecessary suffixes a couple of years ago then.

I missed them.

> I'd certainly be happy to drop _all_ suffixes from the 2si
> conversion insns, but I'm afraid you'll tell me that we need to keep
> what we allow for right now in the assembler. Under such

We need suffix in assembler, but we can drop it from disassembler.
We shouldn't add new suffixes in assembler.

> circumstances, making the disassembler output consistent is going
> to be clumsy at best, but leaving it inconsistent isn't any better.
>
> Jan



-- 
H.J.