[PATCH v2 5/9] x86: improve handling of insns with ambiguous operand sizes

[Jan Beulich]

On 12.11.2019 21:47,  H.J. Lu  wrote:
> On Mon, Nov 11, 2019 at 11:23 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 11.11.2019 19:27,  H.J. Lu  wrote:
>>> On Mon, Nov 11, 2019 at 1:29 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 08.11.2019 16:54,  H.J. Lu  wrote:
>>>>> On Fri, Nov 8, 2019 at 12:09 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> On 07.11.2019 18:47,  H.J. Lu  wrote:
>>>>>>> I don't think DefaultSize matters for them either in AT&T syntax.
>>>>>>> and I don't think we should add DefaultSize to more instructions.
>>>>>>
>>>>>> Then _again_ - what is your alternative suggestion?
>>>>>
>>>>> Don't add DefaultSize to more instructions.
>>>>
>>>> So I think I've recalled meanwhile: The issue is with us not wanting
>>>> to issue diagnostics on e.g. LGDT despite it allowing multiple
>>>> operand sizes. Instead, just like for PUSH/POP etc we want to silently
>>>> default to the most appropriate operand size for the mode. Hence
>>>> adding DefaultSize seems very applicable to me here.
>>>>
>>>
>>> This is a problem only for Intel syntax and DefaultSize affects both
>>> Intel syntax and AT&T syntax.
>>
>> Are you talking about before or after this patch? Recall that the
>> changes here bring AT&T syntax handling more in line with Intel
>> one, just that for AT&T talk is about absent suffixes while for
>> Intel talk is about absent operand size modifiers.
>>
>>>  Since we can't have both DefaultSize
>>> and IgnoreSize at the same time, can they be merged and leave it
>>> for Intel syntax to deal with it?
>>
>> I don't think they can be merged. See the draft patch you've sent
>> the other day actually splitting DefaultSize into two bits.
>>
> 
> The current code has
> 
>   else if (i.tm.opcode_modifier.defaultsize
>            && !i.suffix
>            /* exclude fldenv/frstor/fsave/fstenv */
>            && i.tm.opcode_modifier.no_ssuf)
>     {
>       if (stackop_size == LONG_MNEM_SUFFIX
>           && i.tm.base_opcode == 0xcf)
>         {
>           /* stackop_size is set to LONG_MNEM_SUFFIX for the
>              .code16gcc directive to support 16-bit mode with
>              32-bit address.  For IRET without a suffix, generate
>              16-bit IRET (opcode 0xcf) to return from an interrupt
>              handler.  */
>           i.suffix = WORD_MNEM_SUFFIX;
>           as_warn (_("generating 16-bit `iret' for .code16gcc directive"));
>         }
>       else
>         i.suffix = stackop_size;
>     }
>   else if (intel_syntax
>            && !i.suffix
>            && (i.tm.operand_types[0].bitfield.jumpabsolute
>                || i.tm.opcode_modifier.jumpbyte
>                || i.tm.opcode_modifier.jumpintersegment
>                || (i.tm.base_opcode == 0x0f01 /* [ls][gi]dt */
>                    && i.tm.extension_opcode <= 3)))
>     {
> 
> What you did was to move Intel syntax specific check to generic.
> We shouldn't do that.

Why not? That's the main purpose of this patch. (FAOD it's not the
"move Intel syntax specific check to generic" aspect as such, but
the improved checking in AT&T syntax. It just so happens that this
is easiest expressed by folding the logic.)

Jan