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

[H.J. Lu]

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.


-- 
H.J.