melver added a comment.

In D103958#2809353 <https://reviews.llvm.org/D103958#2809353>, @efriedma wrote:

> You could break `__builtin_load_with_control_dependency(x)` into something 
> like `__builtin_control_dependency(READ_ONCE(x))`.  I don't think any 
> transforms will touch that in practice, even if it isn't theoretically sound. 
>  The rest of my suggestion still applies to that form, I think.  They key 
> point is that the compiler just needs to ensure some branch consumes the 
> loaded value; it doesn't matter which branch it is.

Because the original inline asm version was pretty similar (they just called it 
`volatile_cond()`), I think `__builtin_control_dependency()` is equivalent. 
Actually a later suggestion just called it `__builtin_ctrl_depends()`: 
https://lkml.kernel.org/r/yl9teqealhxbb...@hirez.programming.kicks-ass.net

It was nacked by GCC devs, who expressed concern that it seems impossible to 
guarantee a branch is emitted but also way too difficult to specify. The 
emitting-branch part seems straightforward, as you suggested.

I think implementation-wise, we can probably use your idea either way. I just 
worry about defined semantics, see below.

> The theoretical problem with separating the load from the branch is that it 
> imposes an implicit contract: the branch has to use the value of the load as 
> input, not an equivalent value produced some other way.  This is the general 
> problem with C++11 consume ordering, which nobody has tried to tackle.

Indeed. Which is why I wanted to steer clear of a __builtin that talks about 
control-dependencies directly. There are 2 challenges:

1. Defining the value used to establish a control dependency, e.g. the load 
later writes depend on (kernel only defines writes to be ctrl-dependently 
ordered).
2. Defining later ops that are control-dependent. With an expression like the 
__builtin, this could be any operation after, or it becomes too hard to define:

  x = __builtin_control_dependency(expr); // __builtin_control_dependency 
establishes an ordering edge between loads in expr and later ops
  y = 1; // control dependently ordered, although there is no explicit control 
statement yet...
  if (x) { z = 1; } // ... this code is only interested in z=1 to be 
control-dependently ordered.

Both are hard to define, as you suggested it's similar to consume which was 
also my worry.

Therefore, to get something simple that works and isn't doomed to a definition 
that is unimplementable, I tried to just talk about the control statement and 
the fact a branch must be emitted. In theory, (1) might still be a problem, but 
in practice the compiler has no other way than to use the loaded value if that 
value was loaded through an __atomic or volatile or similar.

Limiting ourselves to an attribute on control statements solves (2), because we 
can say that "the conditional branch is placed *before* conditionally executed 
instructions". Either that, or we make the __builtin give an error if used 
outside the condition of a control statement.

----

Given we've already gotten this far, I will summarize the options:

A. `__builtin_load_with_control_dependency()` -- this appears to solve the 
problem (1) above, but not (2). This approach seems unappealing if we want to 
solve this for the Linux kernel, because the whole point of compiler support 
was to avoid more read-primitives (with new primitives they say they'd just 
upgrade these to acquire and be done with it).

B. `__builtin_control_dependency()` -- would be nice if this would work, but I 
think it suffers from problems (1), and (2) above and is very hard to define 
properly.
B1 <https://reviews.llvm.org/B1>. Do this but constrain it to only be usable as 
conditions in control statements, which would solve (2) at least.

C. `[[mustcontrol]]`:

  Marking a conditional control statement as ``mustcontrol`` indicates that the
  compiler must generate a conditional branch in machine code, and such
  conditional branch is placed *before* conditionally executed instructions. The
  attribute may be ignored if the condition of the control statement is a
  constant expression.

D. But we can also just rename it to `[[control_dependency]]` if that's 
clearer. It looks like it's the same as B1 <https://reviews.llvm.org/B1> minus 
the artificial constraint; implementation should be similar. It'd allow for the 
same arch-dependent omission if an arch does not care about control 
dependencies. But I feel that, at least for the Linux kernel, they prefer 
having as much control over codegen as possible, regardless of arch. Because if 
there's an arch-agnostic way of doing this, we get it for free for POWER and 
Armv7.

All we'd like is a robust primitive, without overcomplicating things.

What do you recommend?

Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103958/new/

https://reviews.llvm.org/D103958

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to