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

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

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

Jan