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

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

On Mon, Sep 17, 2018 at 6:15 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> 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.

No one is expecting that every commit is 100% perfect.  I am working on
WIG issues on users/hjl/wig branch.  Please let me know if you have any
specific issues with my commits on users/hjl/wig branch.

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

I am trying.

> 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).
>

I have been trying to resolve controversy asap.

BTW, it is encouraged to open a bug report for issues with user impact.

-- 
H.J.