Re: [PATCH] x86: fix AVX-512 16-bit addressing

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

>>> On 22.11.17 at 16:52, <hjl.tools@gmail.com> wrote:
> On Wed, Nov 22, 2017 at 7:46 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 22.11.17 at 16:36, <hjl.tools@gmail.com> wrote:
>>> On Wed, Nov 22, 2017 at 6:18 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 22.11.17 at 14:51, <hjl.tools@gmail.com> wrote:
>>>>> On Wed, Nov 22, 2017 at 5:26 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>>> On 21.11.17 at 20:06, <hjl.tools@gmail.com> wrote:
>>>>>>> On Mon, Nov 20, 2017 at 7:15 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> --- a/gas/config/tc-i386.c
>>>>>>>> +++ b/gas/config/tc-i386.c
>>>>>>>> @@ -4799,11 +4799,9 @@ check_VecOperands (const insn_template *
>>>>>>>>                 else
>>>>>>>>                   {
>>>>>>>>                     /* Vector insn can only have Vec_Disp8/Disp32 in
>>>>>>>> -                      32/64bit modes, and Vec_Disp8/Disp16 in 16bit
>>>>>>>> -                      mode.  */
>>>>>>>> +                      64bit mode, and Vec_Disp8/Disp16/Disp32 in 16/32bit
>>>>>>>> +                      modes.  */
>>>>>>>
>>>>>>> Do we really support 32-bit displacement in 16-bit mode or with 0x67
>>>>>>> address prefix?
>>>>>>
>>>>>> What a strange question: Of course we do, and why would we not?
>>>>>> Using 32-bit addresses in 16-bit mode is quite useful, and using 16-bit
>>>>>> addresses in 32-bit mode is at least not illegal. And quite obviously
>>>>>> there should be no difference between EVEX encoded insns and any
>>>>>> other ones.
>>>>>
>>>>> We need tests for 32-bit displacement with and without relocations for
>>>>> .code16 as well as 0x67 prefix.
>>>>
>>>> You're kidding? The patch doesn't change the behavior of .code16
>>>> at all! All it changes is the behavior for .code32 when using 16-bit
>>>> addressing (assembler) and 16-bit code _without_ the 0x67 prefix
>>>> (disassembler). If you had cared about testing what you say above,
>>>> you should have long added a test case.
>>>>
>>>> Please can you stop inventing requirements for bug fixes to go in?
>>>
>>> If we don't know for sure that Disp32 works correctly for 16-bit code,
>>> we shouldn't allow Disp32 for 16-bit instructions.
>>
>> That's a possibility (albeit a bad one, and I would veto a such a
>> change if it affected Intel syntax mode as well), yet once again -
>> it's completely orthogonal to what the patch does. The comment
>> above that I'm changing simply is not true from an architecture
>> perspective, no matter what the assembler actually means to
>> support. But if you prefer I can leave the comment alone
>> (ignoring the fact that it's then becoming not only wrong, but
>> also stale) and remove only the two lines of code which cause
>> the bug.
> 
> I'd like to try your patch.  But your mailer mangled it.   It will
> be easier for me if you create a git branch for your change.

Here it is as an attachment (albeit I'm sending many patches to
other lists, and nowadays they make through unmangled there).
I'm not enough into git to start playing with branches.

Jan

Attachment: binutils-master-x86-AVX512-16bit-addressing.patch
Description: Text document