linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: will@kernel.org, paulmck@kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	stern@rowland.harvard.edu, parri.andrea@gmail.com,
	boqun.feng@gmail.com, npiggin@gmail.com, dhowells@redhat.com,
	j.alglave@ucl.ac.uk, luc.maranget@inria.fr, akiyks@gmail.com,
	linux-toolchains@vger.kernel.org, linux-arch@vger.kernel.org
Subject: Re: [RFC PATCH] LKMM: Add ctrl_dep() macro for control dependency
Date: Wed, 29 Sep 2021 16:47:03 -0500	[thread overview]
Message-ID: <20210929214703.GG22689@gate.crashing.org> (raw)
In-Reply-To: <20210928211507.20335-1-mathieu.desnoyers@efficios.com>

Hi!

On Tue, Sep 28, 2021 at 05:15:07PM -0400, Mathieu Desnoyers wrote:
> C99 describes that accessing volatile objects are side-effects, and that
> "at certain specified points in the execution sequence called sequence
> points, all side effects of previous evaluations shall be complete
> and no side effects of subsequent evaluations shall have taken
> place". [2]

But note that the kernel explicitly uses C89 (with GNU extensions).
Side effects are largely equal there though.

Also note that there may no place in the generated machine code that
corresponds exactly to some sequence point.  Sequence points are a
concept that applies to the source program and how that executes on the
abstract machine.

> +Because ctrl_dep emits distinct asm volatile within each leg of the if
> +statement, the compiler cannot transform the two writes to 'b' into a
> +conditional-move (cmov) instruction, thus ensuring the presence of a
> +conditional branch.  Also because the ctrl_dep emits asm volatile within
> +each leg of the if statement, the compiler cannot move the write to 'c'
> +before the conditional branch.

I think your reasoning here misses some things.  So many that I don't
know where to start to list them, every "because" and "thus" here does
not follow, and even the statements of fact are not a given.

Why do you want a conditional branch insn at all, anyway?  You really
want something else as far as I can see.

It is essential here that there is a READ_ONCE and the WRITE_ONCE.
Those things might make it work the way you want, but as Linus says this
is all way too subtle.  Can you include the *_ONCE into the primitive
itself somehow?


Segher

  parent reply	other threads:[~2021-09-29 21:53 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28 21:15 [RFC PATCH] LKMM: Add ctrl_dep() macro for control dependency Mathieu Desnoyers
2021-09-29 12:06 ` Marco Elver
2021-10-01 15:45   ` Mathieu Desnoyers
2021-10-01 16:20     ` Linus Torvalds
2021-10-01 17:28       ` Mathieu Desnoyers
2021-10-01 18:18         ` Linus Torvalds
2021-09-29 12:28 ` Florian Weimer
2021-09-29 17:41   ` Segher Boessenkool
2021-09-29 19:46     ` Florian Weimer
2021-10-01 16:13     ` Mathieu Desnoyers
2021-10-01 16:26       ` Florian Weimer
2021-10-01 16:35         ` Linus Torvalds
2021-10-10 14:02           ` Florian Weimer
2021-10-14  0:01             ` Paul E. McKenney
2021-10-14  2:14               ` Alan Stern
2021-10-14 16:14                 ` Paul E. McKenney
2021-10-14 15:58               ` Florian Weimer
2021-10-14 16:23                 ` Paul E. McKenney
2021-10-14 18:19                   ` Florian Weimer
2021-10-14 21:09                     ` Paul E. McKenney
2021-10-14 22:36                       ` Linus Torvalds
2021-09-30 13:28   ` Mathieu Desnoyers
2021-09-29 14:47 ` Linus Torvalds
2021-09-29 14:54   ` Linus Torvalds
2021-09-29 19:50     ` Mathieu Desnoyers
2021-09-29 20:13       ` Mathieu Desnoyers
2021-09-29 19:27   ` Mathieu Desnoyers
2021-09-29 22:14     ` Linus Torvalds
2021-09-29 21:47 ` Segher Boessenkool [this message]
2021-09-29 23:57   ` Paul E. McKenney
2021-10-01 15:28     ` Mathieu Desnoyers
2021-10-01 22:53       ` Paul E. McKenney
2021-10-01 19:10     ` Segher Boessenkool
2021-10-01 22:50       ` Paul E. McKenney
2021-10-02 14:29       ` Alan Stern

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=20210929214703.GG22689@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=akiyks@gmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=j.alglave@ucl.ac.uk \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=luc.maranget@inria.fr \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=npiggin@gmail.com \
    --cc=parri.andrea@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stern@rowland.harvard.edu \
    --cc=torvalds@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).