linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: Will Deacon <will@kernel.org>
Cc: Dave Martin <Dave.Martin@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Jason Wang <jasowang@redhat.com>,
	virtualization@lists.linux-foundation.org,
	Arnd Bergmann <arnd@arndb.de>,
	Alan Stern <stern@rowland.harvard.edu>,
	Sami Tolvanen <samitolvanen@google.com>,
	Matt Turner <mattst88@gmail.com>,
	Android Kernel Team <kernel-team@android.com>,
	Kees Cook <keescook@chromium.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Josh Triplett <josh@joshtriplett.org>,
	Ivan Kokshaysky <ink@jurassic.park.msu.ru>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Richard Henderson <rth@twiddle.net>,
	Nick Desaulniers <ndesaulniers@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-alpha@vger.kernel.org
Subject: Re: [PATCH 18/18] arm64: lto: Strengthen READ_ONCE() to acquire when CLANG_LTO=y
Date: Mon, 6 Jul 2020 21:23:26 +0200	[thread overview]
Message-ID: <CANpmjNP9pb4_2S8sf+Ty2ZqtMxWSrBEosDx2wuL2OQnA9YFt9A@mail.gmail.com> (raw)
In-Reply-To: <20200706183542.GB23766@willie-the-truck>

On Mon, 6 Jul 2020 at 20:35, Will Deacon <will@kernel.org> wrote:
> On Mon, Jul 06, 2020 at 05:00:23PM +0100, Dave Martin wrote:
> > On Thu, Jul 02, 2020 at 08:23:02AM +0100, Will Deacon wrote:
> > > On Wed, Jul 01, 2020 at 06:07:25PM +0100, Dave P Martin wrote:
> > > > Also, can you illustrate code that can only be unsafe with Clang LTO?
> > >
> > > I don't have a concrete example, but it's an ongoing concern over on the LTO
> > > thread [1], so I cooked this to show one way we could deal with it. The main
> > > concern is that the whole-program optimisations enabled by LTO may allow the
> > > compiler to enumerate possible values for a pointer at link time and replace
> > > an address dependency between two loads with a control dependency instead,
> > > defeating the dependency ordering within the CPU.
> >
> > Why can't that happen without LTO?
>
> It could, but I'd argue that it's considerably less likely because there
> is less information available to the compiler to perform these sorts of
> optimisations. It also doesn't appear to be happening in practice.
>
> The current state of affairs is that, if/when we catch the compiler
> performing harmful optimistations, we look for a way to disable them.
> However, there are good reasons to enable LTO, so this is one way to
> do that without having to worry about the potential impact on dependency
> ordering.

If it's of any help, I'll see if we can implement that warning in LLVM
if data dependencies somehow disappear (although I don't have any
cycles to pursue right now myself). Until then, short of manual
inspection or encountering a bug in the wild, there is no proof any of
this happens or doesn't happen.

Also, as some anecdotal evidence it's extremely unlikely, even with
LTO: looking at the passes that LLVM runs, there are a number of
passes that seem to want to eliminate basic blocks, thereby getting
rid of branches. Intuitively, it makes sense, because branches are
expensive on most architectures (for GPU targets, I think it tries
even harder to get rid of branches). If we extend our reasoning and
assumptions of LTO's aggressiveness in that direction, we might
actually end up with fewer branches. That might be beneficial for the
data dependencies we worry about (but not so much for control
dependencies we want to keep). Still, no point in speculating (no pun
intended) until we have hard data what actually happens. :-)

Thanks,
-- Marco

  reply	other threads:[~2020-07-06 19:23 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-30 17:37 [PATCH 00/18] Allow architectures to override __READ_ONCE() Will Deacon
2020-06-30 17:37 ` [PATCH 01/18] tools: bpf: Use local copy of headers including uapi/linux/filter.h Will Deacon
2020-07-01 16:38   ` Alexei Starovoitov
2020-06-30 17:37 ` [PATCH 02/18] compiler.h: Split {READ,WRITE}_ONCE definitions out into rwonce.h Will Deacon
2020-06-30 19:11   ` Arnd Bergmann
2020-07-01 10:16     ` Will Deacon
2020-07-01 11:33       ` Arnd Bergmann
2020-06-30 17:37 ` [PATCH 03/18] asm/rwonce: Allow __READ_ONCE to be overridden by the architecture Will Deacon
2020-06-30 17:37 ` [PATCH 04/18] alpha: Override READ_ONCE() with barriered implementation Will Deacon
2020-07-02  9:32   ` Mark Rutland
2020-07-02  9:48     ` Will Deacon
2020-07-02 10:08       ` Arnd Bergmann
2020-07-02 11:18         ` Will Deacon
2020-07-02 11:39           ` Arnd Bergmann
2020-07-02 14:43   ` Joel Fernandes
2020-07-02 14:55     ` Will Deacon
2020-07-02 15:07       ` Joel Fernandes
2020-06-30 17:37 ` [PATCH 05/18] asm/rwonce: Remove smp_read_barrier_depends() invocation Will Deacon
2020-06-30 17:37 ` [PATCH 06/18] vhost: Remove redundant use of read_barrier_depends() barrier Will Deacon
2020-06-30 17:37 ` [PATCH 07/18] alpha: Replace smp_read_barrier_depends() usage with smp_[r]mb() Will Deacon
2020-06-30 17:37 ` [PATCH 08/18] locking/barriers: Remove definitions for [smp_]read_barrier_depends() Will Deacon
2020-06-30 17:37 ` [PATCH 09/18] Documentation/barriers: Remove references to [smp_]read_barrier_depends() Will Deacon
2020-06-30 17:37 ` [PATCH 10/18] Documentation/barriers/kokr: " Will Deacon
2020-06-30 17:37 ` [PATCH 11/18] tools/memory-model: Remove smp_read_barrier_depends() from informal doc Will Deacon
2020-06-30 17:37 ` [PATCH 12/18] include/linux: Remove smp_read_barrier_depends() from comments Will Deacon
2020-06-30 17:37 ` [PATCH 13/18] checkpatch: Remove checks relating to [smp_]read_barrier_depends() Will Deacon
2020-06-30 17:37 ` [PATCH 14/18] arm64: Reduce the number of header files pulled into vmlinux.lds.S Will Deacon
2020-06-30 17:37 ` [PATCH 15/18] arm64: alternatives: Split up alternative.h Will Deacon
2020-06-30 17:37 ` [PATCH 16/18] arm64: cpufeatures: Add capability for LDAPR instruction Will Deacon
2020-06-30 17:37 ` [PATCH 17/18] arm64: alternatives: Remove READ_ONCE() usage during patch operation Will Deacon
2020-06-30 17:37 ` [PATCH 18/18] arm64: lto: Strengthen READ_ONCE() to acquire when CLANG_LTO=y Will Deacon
2020-06-30 19:25   ` Arnd Bergmann
2020-07-01 10:19     ` Will Deacon
2020-07-01 10:59       ` Arnd Bergmann
2020-06-30 19:47   ` Marco Elver
2020-06-30 20:20     ` Peter Zijlstra
2020-06-30 22:57     ` Sami Tolvanen
2020-07-01 10:25       ` Will Deacon
2020-07-01 10:24     ` Will Deacon
2020-07-01 17:07   ` Dave P Martin
2020-07-02  7:23     ` Will Deacon
2020-07-06 16:00       ` Dave Martin
2020-07-06 16:34         ` Paul E. McKenney
2020-07-06 17:05           ` Dave Martin
2020-07-06 17:36             ` Paul E. McKenney
2020-07-07 10:29               ` Dave Martin
2020-07-07 22:51                 ` Paul E. McKenney
2020-07-07 23:01                   ` Nick Desaulniers
2020-07-08  7:15                     ` Marco Elver
2020-07-08  9:16                     ` Peter Zijlstra
2020-07-08 18:20                       ` Paul E. McKenney
2020-07-06 18:35         ` Will Deacon
2020-07-06 19:23           ` Marco Elver [this message]
2020-07-06 19:42             ` Paul E. McKenney
2020-07-06 16:08   ` Dave Martin
2020-07-06 18:35     ` Will Deacon
2020-07-07 10:10       ` Dave Martin
2020-07-01  7:38 ` [PATCH 00/18] Allow architectures to override __READ_ONCE() Josh Triplett

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CANpmjNP9pb4_2S8sf+Ty2ZqtMxWSrBEosDx2wuL2OQnA9YFt9A@mail.gmail.com \
    --to=elver@google.com \
    --cc=Dave.Martin@arm.com \
    --cc=arnd@arndb.de \
    --cc=boqun.feng@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=ink@jurassic.park.msu.ru \
    --cc=jasowang@redhat.com \
    --cc=josh@joshtriplett.org \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mattst88@gmail.com \
    --cc=mst@redhat.com \
    --cc=ndesaulniers@google.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rth@twiddle.net \
    --cc=samitolvanen@google.com \
    --cc=stern@rowland.harvard.edu \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).