Re: [PATCH v2 2/8] x86: Add COND_VZEROUPPER that can replace vzeroupper if no `ret`

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Fri, Jun 3, 2022 at 6:12 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jun 3, 2022 at 1:04 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >
> > The RTM vzeroupper mitigation has no way of replacing inline
> > vzeroupper not before a return.
> >
> > This code does not change any existing functionality.
> >
> > There is no difference in the objdump of libc.so before and after this
> > patch.
> > ---
> >  sysdeps/x86_64/multiarch/avx-rtm-vecs.h  |  1 +
> >  sysdeps/x86_64/multiarch/avx2-rtm-vecs.h |  1 +
> >  sysdeps/x86_64/sysdep.h                  | 16 ++++++++++++++++
> >  3 files changed, 18 insertions(+)
> >
> > diff --git a/sysdeps/x86_64/multiarch/avx-rtm-vecs.h b/sysdeps/x86_64/multiarch/avx-rtm-vecs.h
> > index c00b83ea0e..e954b8e1b0 100644
> > --- a/sysdeps/x86_64/multiarch/avx-rtm-vecs.h
> > +++ b/sysdeps/x86_64/multiarch/avx-rtm-vecs.h
> > @@ -20,6 +20,7 @@
> >  #ifndef _AVX_RTM_VECS_H
> >  #define _AVX_RTM_VECS_H                        1
> >
> > +#define COND_VZEROUPPER                        COND_VZEROUPPER_XTEST
> >  #define ZERO_UPPER_VEC_REGISTERS_RETURN        \
> >         ZERO_UPPER_VEC_REGISTERS_RETURN_XTEST
> >
> > diff --git a/sysdeps/x86_64/multiarch/avx2-rtm-vecs.h b/sysdeps/x86_64/multiarch/avx2-rtm-vecs.h
> > index a5d46e8c66..e20c3635a0 100644
> > --- a/sysdeps/x86_64/multiarch/avx2-rtm-vecs.h
> > +++ b/sysdeps/x86_64/multiarch/avx2-rtm-vecs.h
> > @@ -20,6 +20,7 @@
> >  #ifndef _AVX2_RTM_VECS_H
> >  #define _AVX2_RTM_VECS_H                       1
> >
> > +#define COND_VZEROUPPER                        COND_VZEROUPPER_XTEST
> >  #define ZERO_UPPER_VEC_REGISTERS_RETURN        \
> >         ZERO_UPPER_VEC_REGISTERS_RETURN_XTEST
> >
> > diff --git a/sysdeps/x86_64/sysdep.h b/sysdeps/x86_64/sysdep.h
> > index f14d50786d..2cb31a558b 100644
> > --- a/sysdeps/x86_64/sysdep.h
> > +++ b/sysdeps/x86_64/sysdep.h
> > @@ -106,6 +106,22 @@ lose:                                                                            \
> >         vzeroupper;                                             \
> >         ret
> >
> > +/* Can be used to replace vzeroupper that is not directly before a
> > +   return.  */
> > +#define COND_VZEROUPPER_XTEST                                                  \
> > +    xtest;                                                     \
> > +    jz 1f;                                                     \
> > +    vzeroall;                                                  \
> > +    jmp 2f;                                                    \
> > +1:                                                     \
> > +    vzeroupper;                                                        \
> > +2:
>
> Will "ret" always be after "2:"?

At some point but not immediately afterwards.

For example:

L(zero):
xorl %eax, %eax
VZEROUPPER_RETURN

L(check):
tzcntl %eax, %eax
cmpl %eax, %edx
jle L(zero)
addq %rdi, %rax
VZEROUPPER_RETURN

Can become:

L(zero):
xorl %eax, %eax
ret

L(check):
tzcntl %eax, %eax
COND_VZEROUPPER
cmpl %eax, %edx
jle L(zero)
addq %rdi, %rax
ret

Which saves code size.

>
> > +/* In RTM define this as COND_VZEROUPPER_XTEST.  */
> > +#ifndef COND_VZEROUPPER
> > +# define COND_VZEROUPPER vzeroupper
> > +#endif
> > +
> >  /* Zero upper vector registers and return.  */
> >  #ifndef ZERO_UPPER_VEC_REGISTERS_RETURN
> >  # define ZERO_UPPER_VEC_REGISTERS_RETURN \
> > --
> > 2.34.1
> >
>
>
> --
> H.J.