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

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

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

-- 
H.J.