Re: [PATCH] x86: reject further invalid AVX-512 masking constructs

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

On Wed, Nov 22, 2017 at 5:20 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 22.11.17 at 13:35, <hjl.tools@gmail.com> wrote:
>> On Tue, Nov 21, 2017 at 11:58 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 21.11.17 at 23:18, <hjl.tools@gmail.com> wrote:
>>>> On Mon, Nov 20, 2017 at 6:24 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> On 20.11.17 at 15:10, <hjl.tools@gmail.com> wrote:
>>>>>> On Mon, Nov 20, 2017 at 6:06 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> --- a/gas/testsuite/gas/i386/inval-avx512f.s
>>>>>>> +++ b/gas/testsuite/gas/i386/inval-avx512f.s
>>>>>>> @@ -48,3 +48,6 @@ _start:
>>>>>>>         vaddps zmm2, zmm1, ZMMWORD PTR [eax]{1to16}
>>>>>>>         vaddps zmm2, zmm1, DWORD PTR [eax]
>>>>>>>         vaddpd zmm2, zmm1, QWORD PTR [eax]
>>>>>>> +
>>>>>>> +       vaddps zmm2{ecx}, zmm1, zmm0
>>>>>>> +       vaddps zmm2{z}, zmm1, zmm0
>>>>>>
>>>>>> Do they fail only in Intel syntax?  Testcases in AT&T syntax iare
>>>>>> required unless they are specific to Intel syntax.
>>>>>
>>>>> They fail in both modes. The way the test cases are written which
>>>>> I'm modifying makes it rather ugly to insert AT&T tests. If you
>>>>> want to really force me to do that juggling, may I please ask that
>>>>> on _all_ future tests, line number and section offsets should either
>>>>> be expressed by regex-es or, should their checking be a requirement
>>>>> (like is the case here), Intel and AT&T syntax inputs go into different
>>>>> files so one can easily add to the end of a file without having to flip
>>>>> flop between syntaxes?
>>>>>
>>>>> Please clarify your expectations.
>>>>
>>>> All error checkings should have a testcase in AT&T syntax, unless they
>>>> are specific to Intel syntax.  You can create a new input file to avoid a
>>>> large diff on output of existing test.
>>>
>>> Creating a new input file is as bad as having to fiddle with badly
>>> written ones: Similar type tests really belong together, and a
>>> growing number of test cases has more impact on overall testsuite
>>> execution time than adding a few lines to an existing input. I really
>>> try apply proper reasoning when picking between adding to existing
>>> tests vs introducing new ones. Hence while I'm intending to do what
>>> you ask for here, I at the same time would have hoped for you to
>>> agree to look a little more closely at new tests before allowing them
>>> in, making sure they're written in an extensible way.
>>
>> Do you have any suggestions how to do it?
>
> I've outlined it already: Make output expectations as generic as
> possible, i.e. have them match only what really needs matching
> to be sure the test's purpose is fulfilled (plus of course to avoid
> false matches on other lines, i.e. I'm not suggesting to hide
> register names behind a more generic regexp), while any matching
> output is valid in the given situation.

Sound good.  But I have to see what you got.

>>> And btw, this logically extends to writing test output expectations
>>> in a way to only check for what's really meaningful in a test - going
>>> back to you disagreeing with many of my uses of ? in regex-es. If
>>> you or anyone else cares for e.g. the optional ",1" in an AT&T
>>> memory operand to be produced by the disassembler, this should
>>> be done by a dedicated test. That way, once we can settle on the
>>> disassembler to produce more readable output (just like by default
>>> it omits suffixes, despite them being no more or less optional than
>>> that scaling specifier), it would be exactly one place in the test
>>> suite which needs modifying (be it by adding a command line option
>>> or by adjusting expected output).
>>
>> I prefer not to change disassembler output unless we have a very
>> good reason/
>
> Well, this has happened in a number of times in the past, and I'm
> not sure whether the reasons always were "very good". Plus, just
> to clarify, if I ever manage to get to it, I'm very certainly planning
> to have the disassembler no longer emit "DWORD PTR" and alike in
> Intel syntax mode when it's not required to disambiguate the
> output, just like AT&T suffixes aren't always being output. They're
> making the output unnecessarily hard to read.

This is a good reason to change.

-- 
H.J.