Re: [PATCH 0/2] x86: Fix the -mevexwig=1 assembler option

["Jan Beulich" <JBeulich at suse dot com>]

>>> On 17.09.18 at 14:46, <hjl.tools@gmail.com> wrote:
> On Mon, Sep 17, 2018 at 5:43 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 17.09.18 at 14:31, <hjl.tools@gmail.com> wrote:
>>> On Mon, Sep 17, 2018 at 5:14 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 16.09.18 at 14:15, <hjl.tools@gmail.com> wrote:
>>>>> On Sun, Sep 16, 2018 at 2:38 AM, Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>> "H.J. Lu" <hjl.tools@gmail.com> 09/14/18 9:14 PM >>>
>>>>>>>The VEX.W/EVEX.W bit is ignored by some VEX/EVEX instructions, aka WIG
>>>>>>>instructions.  The -mevexwig=1 assembler option assumes that if the
>>>>>>>vexw field of an VEX/EVEX instruction is 0, it is a WIG instruction.
>>>>>>>But the vexw field of some non-WIG VEX/EVEX instructions is 0 and their
>>>>>>>VEX.W/EVEX.W bit is determined by the integer register operand size.  This
>>>>>>>patch set adds VEXWIG, defined as 3, to indicate that the VEX.W/EVEX.W
>>>>>>>bit is ignored and set VexW=3 on VEX/EVEX WIG instructions.
>>>>>>>
>>>>>>>H.J. Lu (2):
>>>>>>>x86: Support VEX/EVEX WIG encoding
>>>>>>>x86: Check non-WIG EVEX instruction encoding with -mevexwig=1
>>>>>>
>>>>>> Furthermore you don't seem to correct/adjust here what you've done in commit
>>>>>> 41d1ab6a6, despite me having said so right in reply to that earlier change.
>>> I
>>>>>> continue to think that it would have been best to revert that commit (but
>>> not its
>>>>>> amendment), adding VexWIG here as appropriate, but at the same time not
>>>>>> corrupting cases where the field should indeed remain to be zero, i.e. when
>>>>>> VEX.W is supposed to be determined from integer register width (i.e. REX.W).
>>>>>
>>>>> Please take a look at users/hjl/wig branch and tell me which one needs to
>>>>> fix with a testcase.
>>>>
>>>> I'm afraid I don't understand your request. I've already answered the
>>>> "which ones need to be fixed" part: Said commit should simply be
>>>> reverted; only the add-on commit 5be12fc1ad should stay (by these
>>>> get converted to VexWIG now anyway). And I can't make sense of the
>>>> "with a testcase" part - it's mainly up to you if and where you think a
>>>> testcase is needed (because I think existing ones already cover this).
>>>
>>> Are there any specific issues on users/hjl/wig branch? If yes, please show
>>> me a testcas which isn't handled properly.
>>
>> The issue isn't that something wouldn't work, but that VexW=1 / VexW=2
>> settings there are as wrong (due to being misleading and inconsistent
> 
> Please open a bug report with assembly testcases.  If there are no such
> assembly tests, there are no issues here.

Which means you think that random pointless attributes are fine to have?
And violating what is being said elsewhere in the code is fine, too? Excuse
me, but no. That earlier change of yours didn't, afaict, fix any misbehavior
either, or else you'd have added a new testcase there. By what you tell
me (no brokenness -> no change) your patch shouldn't have gone in, nor
should any changes whatsoever go in which solely clean up or simplify
code. This makes no sense at all.

May I once again ask that you apply the same criteria to your own work
that you apply to other people's contributions?

I realize that the binutils project's policy is more lax than that of other
projects, not requiring maintainers to wait for any form of review, but I
think it is pretty bad practice to abuse this power in case of controversy
(up to and including to suppress controversy by simply committing things
without any delay, even if changes could be controversial).

Jan

>> with the comment in i386-opc.h) as the bazillions of IgnoreSize were (just
>> to give an example to compare with). There should not ever be attributes
>> which aren't necessary, or else people like me looking at the table will
>> endlessly wonder "Why's this attribute here?" For a job like the template
>> folding (which now looks to be mostly done), stray attributes were also a
>> significant hindrance, as them pointlessly being different between
>> templates made it harder to reason whether templates could be folded.
>>
>> Jan
>>
>>
> 
> 
> 
> -- 
> H.J.