This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH 2/5] x86: Add -mindirect-branch-loop=


Hi All, 

> -----Original Message-----
> From: H.J. Lu [mailto:hjl.tools@gmail.com]
> Sent: Saturday, January 13, 2018 1:11 AM
> To: Kumar, Venkataramanan <Venkataramanan.Kumar@amd.com>
> Cc: Nagarajan, Muthu kumar raj <Muthukumarraj.Nagarajan@amd.com>;
> GCC Patches <gcc-patches@gcc.gnu.org>; Martin Jambor
> <mjambor@suse.cz>; Jeff Law <law@redhat.com>; Uros Bizjak
> (ubizjak@gmail.com) <ubizjak@gmail.com>; Jan Hubicka <jh@suse.de>;
> Dharmakan, Rohit arul raj <Rohitarulraj.Dharmakan@amd.com>
> Subject: Re: [PATCH 2/5] x86: Add -mindirect-branch-loop=
> 
> On Fri, Jan 12, 2018 at 10:27 AM, Kumar, Venkataramanan
> <Venkataramanan.Kumar@amd.com> wrote:
> > Hi HJ,
> >
> >> -----Original Message-----
> >> From: Kumar, Venkataramanan
> >> Sent: Friday, January 12, 2018 8:39 PM
> >> To: 'H.J. Lu' <hjl.tools@gmail.com>; 'Martin Jambor'
> >> <mjambor@suse.cz>
> >> Cc: Nagarajan, Muthu kumar raj <Muthukumarraj.Nagarajan@amd.com>;
> >> 'GCC Patches' <gcc-patches@gcc.gnu.org>; 'Jeff Law' <law@redhat.com>;
> >> Uros Bizjak (ubizjak@gmail.com) <ubizjak@gmail.com>; 'Jan Hubicka'
> >> <jh@suse.de>
> >> Subject: RE: [PATCH 2/5] x86: Add -mindirect-branch-loop=
> >>
> >> Hi all,
> >>
> >> > -----Original Message-----
> >> > From: Kumar, Venkataramanan
> >> > Sent: Friday, January 12, 2018 8:16 PM
> >> > To: 'H.J. Lu' <hjl.tools@gmail.com>; Martin Jambor
> >> > <mjambor@suse.cz>
> >> > Cc: Nagarajan, Muthu kumar raj
> <Muthukumarraj.Nagarajan@amd.com>;
> >> > GCC Patches <gcc-patches@gcc.gnu.org>; Jeff Law <law@redhat.com>;
> >> Uros
> >> > Bizjak (ubizjak@gmail.com) <ubizjak@gmail.com>; 'Jan Hubicka'
> >> > <jh@suse.de>
> >> > Subject: RE: [PATCH 2/5] x86: Add -mindirect-branch-loop=
> >> >
> >> > Hi all,
> >> >
> >> > > -----Original Message-----
> >> > > From: H.J. Lu [mailto:hjl.tools@gmail.com]
> >> > > Sent: Friday, January 12, 2018 7:36 PM
> >> > > To: Martin Jambor <mjambor@suse.cz>
> >> > > Cc: Nagarajan, Muthu kumar raj
> >> <Muthukumarraj.Nagarajan@amd.com>;
> >> > > Kumar, Venkataramanan <Venkataramanan.Kumar@amd.com>; GCC
> >> > Patches
> >> > > <gcc-patches@gcc.gnu.org>; Jeff Law <law@redhat.com>
> >> > > Subject: Re: [PATCH 2/5] x86: Add -mindirect-branch-loop=
> >> > >
> >> > > On Fri, Jan 12, 2018 at 4:38 AM, Martin Jambor <mjambor@suse.cz>
> >> wrote:
> >> > > > Hi,
> >> > > >
> >> > > > On Thu, Jan 11 2018, Jeff Law wrote:
> >> > > >> On 01/07/2018 03:59 PM, H.J. Lu wrote:
> >> > > >>> Add -mindirect-branch-loop= option to control loop filler in
> >> > > >>> call and return thunks generated by -mindirect-branch=.
> >> > > >>> 'lfence' uses
> >> > > "lfence"
> >> > > >>> as loop filler.  'pause' uses "pause" as loop filler.  'nop' uses "nop"
> >> > > >>> as loop filler.  The default is 'lfence'.
> >> > > >>>
> >> > > >>> gcc/
> >> > > >>>
> >> > > >>>      * config/i386/i386-opts.h (indirect_branch_loop): New.
> >> > > >>>      * config/i386/i386.c (output_indirect_thunk): Support
> >> > > >>>      -mindirect-branch-loop=.
> >> > > >>>      * config/i386/i386.opt (mindirect-branch-loop=): New option.
> >> > > >>>      (indirect_branch_loop): New.
> >> > > >>>      (lfence): Likewise.
> >> > > >>>      (pause): Likewise.
> >> > > >>>      (nop): Likewise.
> >> > > >>>      * doc/invoke.texi: Document -mindirect-branch-loop= option.
> >> > > >>>
> >> > > >>> gcc/testsuite/
> >> > > >>>
> >> > > >>>      * gcc.target/i386/indirect-thunk-loop-1.c: New test.
> >> > > >>>      * gcc.target/i386/indirect-thunk-loop-2.c: Likewise.
> >> > > >>>      * gcc.target/i386/indirect-thunk-loop-3.c: Likewise.
> >> > > >>>      * gcc.target/i386/indirect-thunk-loop-4.c: Likewise.
> >> > > >>>      * gcc.target/i386/indirect-thunk-loop-5.c: Likewise.
> >> > > >> I think we should drop the ability to change the filler until
> >> > > >> such time as we really need it.  Just pick one and go with it.
> >> > > >> I think David suggested that they wanted "pause".  I'm
> >> > > >> obviously fine
> >> with that.
> >> > > >>
> >> > > >
> >> > > > unless I am mistaken (which is frankly quite possible, I am
> >> > > > still not quite up to speed about the nuances), AMD strongly
> >> > > > prefers the lfence variant.  OTOH, IIUC, in kernel this will be
> >> > > > run-time patched but so it does not matter in the most pressing
> >> > > > case and we might want to have a mechanism doing something
> >> > > > similar for protecting
> >> > userspace later on.
> >> > > > But perhaps it is enough to keep the option?
> >> > > >
> >> > > > Muthu and/or Venkat, can you please comment?
> >> > >
> >> > > If we do want it, I will submit a separate patch AFTER the
> >> > > current patch set has been approved and checked into GCC 8.
> >> > >
> >> >
> >> > As per AMD architects, using “lfence” in “retpoline” is better than
> >> > “pause” for our targets.
> >> > So please allow filler to use "lfence".
> >> >
> >> We also leant that "lfence" is a dispatch serializing instruction.
> >> The Puse instruction is not serializing on AMD processors and has high
> latencies.
> >>
> > Any reason why Intel has chosen "pause" over "lfence" as the default loop
> filler for Retpoline?
> >
> 
> My original patch uses "lfence".  I was asked to use "pause":
> 
> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00969.html

If everyone is ok, my suggestion is to use  "lfence" as the default loop filler for retpoline.

Please confirm.

> 
> --
> H.J.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]