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

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

On Wed, Nov 13, 2019 at 5:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> 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.)
>

What do you mean by improving AT&T syntax?  AT&T syntax has
many quirks.  We should leave them alone if we can.

-- 
H.J.