Re: [PATCH 6/6] x86: fold RegEip/RegRip and RegEiz/RegRiz

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

On Fri, Aug 3, 2018 at 9:01 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 03.08.18 at 17:33, <hjl.tools@gmail.com> wrote:
>> On Thu, Aug 2, 2018 at 11:55 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 02.08.18 at 18:33, <hjl.tools@gmail.com> wrote:
>>>> On Thu, Aug 2, 2018 at 9:06 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> On 02.08.18 at 14:49, <hjl.tools@gmail.com> wrote:
>>>>>> We get an error today:
>>>>>>
>>>>>> [hjl@gnu-tools-1 tmp]$ cat x.s
>>>>>> bndmk (%eip), %bnd2
>>>>>> [hjl@gnu-tools-1 tmp]$ gcc -c x.s
>>>>>> x.s: Assembler messages:
>>>>>> x.s:1: Error: 32-bit address isn't allowed in 64-bit MPX instructions.
>>>>>> [hjl@gnu-tools-1 tmp]$
>>>>>>
>>>>>> With your change, what do we get?
>>>>>
>>>>> `(%eip)' cannot be used here
>>>>
>>>>  .*  Error: 32-bit address isn't allowed in 64-bit MPX instructions\.
>>>>  [      ]*[1-9][0-9]*[  ]+4C1903
>>>>  [      ]*[1-9][0-9]*[  ]+bndmk \(%rip\), %bnd3
>>>> -[      ]*[1-9][0-9]*[  ]+\?\?\?\? 67F30F1B             bndmk \(%eip\), %bnd2
>>>> -.*  Error: 32-bit address isn't allowed in 64-bit MPX instructions\.
>>>> -[      ]*[1-9][0-9]*[  ]+15000000
>>>> -[      ]*[1-9][0-9]*[  ]+00
>>>> +[      ]*[1-9][0-9]*[  ]+bndmk \(%eip\), %bnd2
>>>>  [      ]*[1-9][0-9]*[  ]+
>>>>  [      ]*[1-9][0-9]*[  ]+\#\#\# bndmov
>>>>  [      ]*[1-9][0-9]*[  ]+\?\?\?\? 6766410F             bndmov \(%r8d\), %bnd1
>>>>
>>>> Why is there no error message for bndmk (%eip), %bnd2"?  Also
>>>
>>> Well, I've explained this already, but maybe the connection isn't
>>> as obvious to you as I thought it would be: _Any_ errors appearing
>>> in the listing are an indication of a problem, because it means the
>>> assembler didn't simply bail after finding an error. In the case here
>>> it is
>>>
>>>   if (i.tm.cpu_flags.bitfield.cpumpx)
>>>     {
>>>       if (flag_code == CODE_64BIT && i.prefix[ADDR_PREFIX])
>>>         as_bad (_("32-bit address isn't allowed in 64-bit MPX instructions."));
>>>       else if (flag_code != CODE_16BIT
>>>                ? i.prefix[ADDR_PREFIX]
>>>                : i.mem_operands && !i.prefix[ADDR_PREFIX])
>>>         as_bad (_("16-bit address isn't allowed in MPX instructions"));
>>>     }
>>>
>>> Note how there no "return" here. That's the very inconsistency I
>>
>> We can add a return here.
>
> And that's my plan going forward, as said.
>
>>> did explain before (see the parenthesized statements in my second
>>> most recent response, still visible in quoted context above), and
>>> that I mean to get to fixing eventually.
>>>
>>> Bottom line - an error disappearing from the listing should not be
>>> considered a problem, but an improvement (the only alternative
>>> view I could accept would be if _all_ errors appeared in the listing).
>>>
>>>> `(%eip)' cannot be used here
>>>>
>>>> contains less info than
>>>>
>>>> 32-bit address isn't allowed in 64-bit MPX instructions
>>>
>>> Why that? Everyone ought to know that %eip is 32 bits. And once
>>> again - it being wrong to use 32-bit _or_ IP-relative addressing,
>>> there shouldn't be a requirement which of the two gets reported.
>>> So in the end this change of error message is because the error
>>> now comes from parse_operands(), in which case md_assemble()
>>> bails rather than reaching the piece of quoted code above. I view
>>> it as entirely unreasonable to complicate the code in
>>> i386_index_check() just to be able to emit that other diagnostic.
>>>
>>
>> We should have the same error for both %eip and %eax for MPX
>> in 64-bit mode.
>
> I'm sorry, but I'm not going to cripple the change just so that your
> unreasonable requirement can be met. Let's take a different example:
>
>         add     (%cr1), (%cr0)
>         add     (,%xmm1), (,%xmm0)
>
> Are you telling me it matters whether the assembler complains about
> the first or second operand being wrong, or there being two memory
> operands when just one is allowed? Once again - when there are
> multiple things wrong for a single insn, there should be no expectation
> whatsoever as to which of the possible error messages actually gets
> emitted, as that's an implementation detail of the assembler. If you
> disagree here, then I'm looking forward for a very good explanation
> of your standpoint.

The 0x67 prefix in MPX instructions is ignored. That is why there is

32-bit address isn't allowed in 64-bit MPX instructions

We should keep this message for "%eip".


-- 
H.J.