Re: [PATCH v7] x86: replace adhoc (partly wrong) ambiguous operand checking for MOVSX/MOVZX

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

On Thu, Feb 13, 2020 at 9:00 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.02.2020 17:57, H.J. Lu wrote:
> > On Thu, Feb 13, 2020 at 8:49 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 13.02.2020 17:39, H.J. Lu wrote:
> >>> On Thu, Feb 13, 2020 at 8:26 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 13.02.2020 17:18, H.J. Lu wrote:
> >>>>> On Thu, Feb 13, 2020 at 7:02 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> On 13.02.2020 15:34, H.J. Lu wrote:
> >>>>>>> On Thu, Feb 13, 2020 at 1:21 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>> @@ -6551,6 +6558,32 @@ process_suffix (void)
> >>>>>>>>         }
> >>>>>>>>      }
> >>>>>>>>
> >>>>>>>> +  if ((i.tm.base_opcode | 8) == 0xfbe
> >>>>>>>> +      || (i.tm.base_opcode == 0x63 && i.tm.cpu_flags.bitfield.cpu64))
> >>>>>>>> +    {
> >>>>>>>> +      /* In Intel syntax, movsx/movzx must have a "suffix" (checked above).
> >>>>>>>> +        In AT&T syntax, if there is no suffix (warned about above), the default
> >>>>>>>> +        will be byte extension.  */
> >>>>>>>
> >>>>>>> Please drop the warning for AT&T syntax and document it instead.
> >>>>>>
> >>>>>> But it is wrong to make a choice silently. When there are multiple
> >>>>>> options, we ought to inform the user, i.e. it needs to be at least
> >>>>>
> >>>>> I have seen
> >>>>>
> >>>>> movzx 7(%eax), %ecx
> >>>>>
> >>>>> from 2010.   We need to live with this oddity of movsx/movzx.
> >>>>
> >>>> Right, hence not an error, but just a warning. Not giving a warning
> >>>
> >>> If a project checks assembler warnings, it will fail to build with
> >>> the new assembler.   We don't want it.
> >>
> >> That's the case for all other new warnings, too. H.J., come on - there
> >> shouldn't be arbitrary / random differences in behavior. Things should
> >> be predictable for people.
> >
> > That is one reason I'd like to see new errors, not new warnings.
>
> I.e. to break the build for everyone when we detect issues in their
> that we previously didn't detect? That's precisely why things should
> be warnings at least initially, to give people time to fix their
> code without forcing them to do so right away.

For this particular case, there is no issue.

-- 
H.J.