[PATCH] x86: Apply standalone prefixes to the following instruction

[H.J. Lu]

On Fri, Jul 19, 2019 at 10:11 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jul 19, 2019 at 9:25 AM Jan Beulich <JBeulich@suse.com> wrote:
> >
> > On 19.07.2019 17:01,  H.J. Lu  wrote:
> > > On Fri, Jul 19, 2019 at 1:46 AM Jan Beulich <JBeulich@suse.com> wrote:
> > >>
> > >> On 19.07.2019 00:26,  H.J. Lu  wrote:
> > >>> Standalone prefixes should be applied to the following instruction,
> > >>> instead of being treated as regular instructions.  An error should be
> > >>> issued when a standalone prefix is at the end of source or isn't
> > >>> followed by an instruction in the same section.
> > >>
> > >> Commenting here, because commenting on the actual code fragments is
> > >> not easily possible with the patch sent as attachment.
> > >>
> > >> For one, I don't agree that errors should be issued when switching
> > >> sections. Clever assembly programming can easily result in the
> > >> actual section later getting resumed, and an appropriate insn being
> > >> there.
> > >
> > > Yes, you will get an error which can be easily fixed.
> >
> > For both this and ...
> >
> > >> And then I'm getting the impression that the change here is going
> > >> to break things like
> > >>
> > >> static inline unsigned int find_first_set_bit(unsigned long word)
> > >> {
> > >>       asm ( "rep; bsf %1,%0" : "=r" (word) : "rm" (word) );
> > >>       return (unsigned int)word;
> > >> }
> > >
> > > "rep; bsf" works like "rep bsf".
> > >
> > >> (quoted from Xen sources), being a backwards compatible
> > >> representation of tzcnt. Just like such have shown up in the past,
> > >> REP prefixes could easily obtain meaning for other insns going
> > >> forward, so tagging individual templates with RepPrefixOk is not
> > >> going to help. WBNOINVD is a pretty recent example.
> > >>
> > >
> > > Since adding REP to random instructions may lead to different instructions,
> > > RepPrefixOk is used to prevent that.  If one really wants different
> > > instructions,
> > > ".byte 0xf3" can be used.
> >
> > ... this you realize that breaking existing code is bad? It doesn't
> > matter how "easy" it is to fix such. Taking Xen (again) as the example,
> > older trees are supposed to not be touched anymore except for security
> > fixes. Now if people upgrade their underlying distros, builds of these
> > older trees will suddenly start to fail.
> >
> > Furthermore, with your ".byte 0xf3" suggestion, what "protection" do
> > you achieve when disallowing "rep", but allowing ".byte 0xf3"? Plus
> > personally I consider the .byte variant quite a bit worse.
>
> It will discourage using old assembler for new instructions.  Of course,
> one can use ".byte" for anything.
>
> > Finally, with a number of changes of mine (including the still pending
> > operand size default changes which I'm slowly making progress with)
> > you've been demanding that the Linux build not be broken. But just
> > like Xen, Linux too uses "rep; bsf".
> >
>
> "rep; bsf" works fine since bsf has RepPrefixOk.  If REP should be
> allowed for any instructions, shouldn't RepPrefixOk be removed?
>

RepPrefixOk was added by

commit 29c048b696d4e93fe9f595d59fcb6239270e5a29
Author: Roland McGrath <roland@gnu.org>
Date:   Fri Jun 22 16:42:08 2012 +0000

    gas/
            * config/tc-i386.c (parse_insn): Don't complain about REP prefix
            when the template has opcode_modifier.repprefixok set.
            * NEWS: Mention the change.

    gas/testsuite/
            * gas/i386/rep-bsf.d: New file.
            * gas/i386/rep-bsf.s: New file.
            * gas/i386/i386.exp: Add the new test.

    opcodes/
            * i386-opc.h (RepPrefixOk): New enum constant.
            (i386_opcode_modifier): New bitfield 'repprefixok'.
            * i386-gen.c (opcode_modifiers): Add RepPrefixOk.
            * i386-opc.tbl: Add RepPrefixOk to bsf, bsr, and to all
            instructions that have IsString.
            * i386-tbl.h: Regenerate.

We can either remove RepPrefixOk or add it to any instructions
which can have REP.

-- 
H.J.