LWN: Comments on "ACCESS_ONCE()"

ACCESS_ONCE()

gmaxwell — Wed, 11 Feb 2015 22:00:41 +0000

Typeof here isn't important to whats being done. Absent typeof you'd just have ACCESS_INT_ONCE() (or use the volatile cast directly instead of via a macro); the use of typeof here just makes the code more tidy and allows using a macro to express the programmer's intent better to other programmers.

ACCESS_ONCE()

cborni — Thu, 04 Dec 2014 20:01:54 +0000

wow, I posted this into the wrong thread. Please disregard this here.

ACCESS_ONCE()

cborni — Thu, 04 Dec 2014 15:27:46 +0000

Actually the old ACCESS_ONCE code was supposed to work:

While it is not that obvious the C99 standard basically forbids
duplicating the memory access in that case. For an argumentation of
a similiar case please see:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=22278#c43

For the implementation-defined cases regarding volatile there are some
GCC-specific clarifications which can be found here:
https://gcc.gnu.org/onlinedocs/gcc/Volatiles.html#Volatiles

This issue was reported as bug and fixed (as a bug)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145"

SO strictly speaking it was a gcc bug

Declaring it volatile

mathstuf — Fri, 31 Jan 2014 04:07:16 +0000

Maybe a better name would be FORCE_ACCESS_HERE().

Declaring it volatile

timur — Thu, 30 Jan 2014 22:55:19 +0000

Hmm ... I think I get it. Since lock->owner is temporarily declared volatile, the compiler forces a load at that point in the code. But 'owner' is not declared volatile, so the compiler doesn't need to reload it, and it has "forgotten" about lock->owner, so it never tries to reload it, until the loop restarts.

Sneaky.

Declaring it volatile

timur — Thu, 30 Jan 2014 22:39:31 +0000

The functionality of this macro is actually well described by its name

I disagree. I was completely confused by the article until I saw the implementation of ACCESS_ONCE, because the behavior being described is the opposite of what I think "access once" means. ACCESS_ONCE makes me believe that that it operates like pr_info_once(), which literally calls pr_info only once, no matter how many times loop repeats. I also don't see how declaring it volatile forces the compiler to reload the variable only at the top of the loop. Since it's declared volatile, the compiler can reload 'owner' any time it wants.

ACCESS_ONCE()

etienne — Mon, 20 Aug 2012 10:07:45 +0000

The thing is, can the type returned by typeof() be modified by adding the "volatile" keyword, how much can it be modified (how to remove volatility, possible to change the sign-ness by adding signed/unsigned)...

ACCESS_ONCE()

PaulMcKenney — Sun, 19 Aug 2012 19:38:21 +0000

ACCESS_ONCE() is simply the macro called out in the article, which simply uses the volatile keyword in a cast, which is part of standard C.

ACCESS_ONCE()

dgm — Fri, 17 Aug 2012 10:57:50 +0000

> the spec says it can safely do so.

But does the spec say it _has_ to do so? Does it more good or harm?

Not everything that is allowed is good. For example, the compiler is (in theory) allowed to do anything it wants when presented with code that raises undefined behavior. Anything. "rm -rf /" for instance would be 100% correct. Are GCC developers planing this "feature" for gcc 4.8? Of course not, that would be stupid when you could be playing nethack instead...

ACCESS_ONCE()

dgm — Fri, 17 Aug 2012 10:37:05 +0000

typeof() is clearly something acceptable in gcc's C dialect. Or have they removed their own extension without the word noticing?

ACCESS_ONCE()

quanstro — Thu, 16 Aug 2012 14:52:04 +0000

i agree with what you say, but that's not the point i'm trying to make.
(and btw, i don't think that ACCESS_ONCE is standard c. nor can the
kernel be compiled with an arbitrary compiler; it depends on gcc.)

for me, making code safe from all possible according-to-hoyle legal
transformations of the code is not really interesting or useful.
i'd much rather focus on having a tractable, easy-to-use programming
environment.

if restricting the compiler from making some theoretically legal
code transformations reduces bugs and generally makes life easier,
then why not do it?

as it is i believe there are some gcc optimizations that can break the
kernel.

ACCESS_ONCE()

jwakely — Mon, 13 Aug 2012 16:58:06 +0000

> Now, gcc has for example the -pthreads switch that tells it something about what to expect so that can help.

Except that for many platforms it's equivalent to '-D_REENTRANT -lpthread' and so only affects the preprocessor and linker, which doesn't tell the compiler anything.

ACCESS_ONCE()

nybble41 — Mon, 13 Aug 2012 16:50:20 +0000

Fortunately, in your example avar is always initialized to 10. :)

Assuming "loglevel = 4" was replaced with "loglevel == 4", I would expect the compiler to generate a warning in this case, since it can't _prove_ that avar was initialized before the printf() call. I would also hope that the compiler would take advantage of the fact that avar is either 10 or undefined to simply set it to 10 regardless of loglevel, for code size and performance reasons if nothing else.

In cases like this, IMHO, it would be better to use a common flag rather than testing loglevel multiple times:

void fct (void) {
int avar;
bool use_avar;
use_avar = (loglevel == 4);
if (use_avar) avar = 10;
increase_loglevel();
do_some_unrelated_stuff();
if (use_avar) printf ("%d\n", avar);
}

I'm not sure whether the compiler can follow this any better than before, so there may still be a warning, but at least the coupling is visible to anyone reading the code, and doesn't depend on the implementation of external functions.

ACCESS_ONCE()

daglwn — Mon, 13 Aug 2012 16:41:56 +0000

> Threads are supposed to share all memory, aren't they?

It depends on the threading model and since the standard doesn't specify one, the compiler cannot know in the general case.

Now, gcc has for example the -pthreads switch that tells it something about what to expect so that can help. But the kernel build system almost certainly doesn't use that.

> There are plenty of opportunities for optimization, but I don't think
> this is an example of one.

When one restricts what is know about memory accesses, the compiler kills a *lot* of compiler transformation. Much much more than one would think. Spill code gets a lot more inefficient, for example, which you wouldn't expect to happen since spills are entirely local. But it does because the compiler cannot do certain peeps that can help reduce the amount of spill code.

The "big" transformations like loop restructuring, vectorization and the like are almost impossible if the behavior of memory can't be determined. These are no more "unsafe" than any other transformation. The compiler won't do them if it doesn't know it's safe according to the standard.

ACCESS_ONCE()

daglwn — Mon, 13 Aug 2012 16:35:25 +0000

Yes, it is. It uses typeof().

But ignoring that, nye has the more useful response. :)

ACCESS_ONCE()

daglwn — Mon, 13 Aug 2012 16:34:21 +0000

It is not trivial to get an exact and accurate answer in the general case, true, but I was assuming we were talking about the common case of locally-declared variables.

Still, even in this case the compiler could warn about it even if it doesn't know for sure. The code certainly looks suspicious and a warning would be appropriate. False positives are just fine if they are limited in number. The compiler can provide directives to suppress them if the programmer knows it's not a problem.

Note that gcc does just this. It warns that variables *might* be uninitialized.

ACCESS_ONCE()

daglwn — Mon, 13 Aug 2012 16:31:38 +0000

Absolutely. One needs to first understand _why_ the warning is there before fixing it. Warnings don't replace understanding.

ACCESS_ONCE()

nye — Mon, 13 Aug 2012 12:11:58 +0000

>> How can the compiler possibly know what you meant when you wrote something outside the language spec?

> The code we are arguing about, the one with ACCESS_ONCE(), is NOT outside the spec in any way, is it?

No it isn't. The point is that, absent the ACCESS_ONCE() macro, the assumption that the compiler shouldn't pull that access out of the loop is what's outside the language spec, because the spec says it can safely do so.

ACCESS_ONCE()

nye — Mon, 13 Aug 2012 12:05:11 +0000

>in the example given in the op, there was no reason for the read to be in the
> loop, except if it might change. the compiler made the assumption that the
> coder was wrong. that might not be a useful assumption.

No, the coder *was* wrong, and the assumption is *always correct in standard C*. That's the point. The programmer might have assumed semantics which are not C, but the compiler merely assumed that the programmer was writing in C, not writing in some unspecified language that looks a lot *like* C and exists only in the programmer's head.

It is axiomatic that a valid optimisation (ie. one which precisely follows C semantics, and any which don't are buggy and tend to be quickly fixed) cannot break correct valid C; if code breaks then it is because the programmer has made incorrect assumptions about the exact meaning *in C* of what they're writing.

If your variable might change between accesses, then you need to tell the compiler that, because it is not the case in the standard C model, which is why there's a keyword existing specifically for that purpose.

ACCESS_ONCE()

dgm — Mon, 13 Aug 2012 10:07:51 +0000

> And any variable might be shared.

Threads are supposed to share all memory, aren't they?

>You've just killed all compiler optimization.

Only the unsafe ones. There are plenty of opportunities for optimization, but I don't think this is an example of one.

ACCESS_ONCE()

dgm — Mon, 13 Aug 2012 09:56:56 +0000

> How can the compiler possibly know what you meant when you wrote something outside the language spec?

The code we are arguing about, the one with ACCESS_ONCE(), is NOT outside the spec in any way, is it?

ACCESS_ONCE()

etienne — Mon, 13 Aug 2012 09:38:32 +0000

It is not trivial, or even possible, to detect if a variable is only initialised and used when another variable is set to a special value.
Something like (very simplified):
extern unsigned loglevel;

void fct (void) {
int avar;
if (loglevel = 4) avar = 10;
increase_loglevel();
do_some_unrelated_stuff();
if (loglevel == 5) printf ("%d\n", avar);
}

ACCESS_ONCE()

jezuch — Mon, 13 Aug 2012 08:02:29 +0000

> C compilers detect it all the time. It is a trivial analysis. Users tend to ignore the warnings, however.

It is. And they do. And every time I see someone fixing a "use of undefined variable" warning by initializing it to zero at declaration point, I cringe. I've seen it in stable updates to the kernel a lot...

ACCESS_ONCE()

Wol — Sun, 12 Aug 2012 18:52:14 +0000

Which is why, when I gave a bunch of novice programmers instruction about how to maintain code, I said "the standard is (a) always turn warnings to max and (b) always fix or otherwise understand *every* warning".

We had a bunch of warnings we couldn't get rid of, hence it didn't say "fix all warnings", but "explain it away" is just as effective, if less satisfying.

Cheers,
Wol

ACCESS_ONCE()

quanstro — Sun, 12 Aug 2012 16:16:03 +0000

in the example given in the op, there was no reason for the read to be in the
loop, except if it might change. the compiler made the assumption that the
coder was wrong. that might not be a useful assumption.

as i see it, the trade-off here is non-standard constructions, and the
principle of least surprise for performance.

i'm not convinced of the claim that this is always a win.

the compiler i use on a daily basis does not do strength reduction or optimize
away indirection. it assumes you know what you're doing. i don't notice that
it is slower. i also don't have to worry that the compiler will break my
drivers by "optimizing" them.

(never mind that with modern intel cpus, strength reduction can be a loss due
to microop caching.)

i think this is a good trade off for my case because it avoids obtuse, and
non-portable constructions that can be hard to remember to apply. that is,
for most code, developer time is more expensive than run time.

just my two cents, and i realize that there are cases in the linux kernel
where complex macros need this sort of optimization. but perhaps that's
complexity begetting itself.

ACCESS_ONCE()

nix — Fri, 10 Aug 2012 17:33:36 +0000

personally, i find this sort of code reorg by compilers to be problematic as it generally makes code quite hard to reason about.

I'm curious. Why don't you say the same about register allocation? Combined with stack spilling, that can often have the same effect as code motion. How do you plan to get rid of that?

(I've seen this from various safety-critical embedded people too: they want the compiler to 'not optimize'. I've tried pointing out that this is a meaningless request, that translation necessarily implies many of the same transformations that optimization does, but they never seem to get it. What they generally *mean* is that they want the smallest possible number of transformations -- or perhaps that they want the transformations to be guaranteed bug-free.)

ACCESS_ONCE()

daglwn — Fri, 10 Aug 2012 15:40:19 +0000

> the claim above is that the choice is to
> (a) follow the standard, or
> (b) do something arbitrary.

This is a false choice. (a) and (b) are the same thing in the presence of undefined/unspecified/implementation-defined behavior. And it's not arbitrary. It's the decision of the compiler engineers.

> personally, i find this sort of code reorg by compilers
> to be problematic as it generally makes code quite hard
> to reason about. experienced developers would lift the
> assignment out of the loop if it mattered, and it were
> legal.

I hear this a lot. Then people get surprised when I show them what the compiler did to their code. Believe me, there is no reason anyone should waste time hand-optimizing code without proof of need. Either they're going to miss a lot of opportunity or they are going to screw things up and make the compiler's job harder.

If a developer were to hand-optimize code to achieve the same performance result, the source code would be unmaintainable.

We want optimizing compilers. I can't believe anyone would suggest otherwise.

ACCESS_ONCE()

daglwn — Fri, 10 Aug 2012 15:35:09 +0000

C compilers detect it all the time. It is a trivial analysis. Users tend to ignore the warnings, however.

ACCESS_ONCE()

quanstro — Fri, 10 Aug 2012 14:58:25 +0000

the claim above is that the choice is to
(a) follow the standard, or
(b) do something arbitrary.

that is not the choice in this case. the choice is to
either
(a) rearrange the code in optimization, or
(b) leave it as the developer wrote it.

personally, i find this sort of code reorg by compilers
to be problematic as it generally makes code quite hard
to reason about. experienced developers would lift the
assignment out of the loop if it mattered, and it were
legal.

ACCESS_ONCE()

jezuch — Fri, 10 Aug 2012 07:40:10 +0000

> Take a very simple but common issue: undefined data.

Undefined data, hah. In Java it is a compilation error to use a variable that is not provably assigned in all possible code paths between declaration and use. I like this feature *a lot* and I'm always surprised that C compilers have such a great difficulty with detecting it. (The JVM verifier does this analysis as well during class loading, so it has to be *fast* as well as correct.)

ACCESS_ONCE()

mmorrow — Thu, 09 Aug 2012 03:47:44 +0000

Take a very simple but common issue: undefined data. There are countless compiler analyses and transformations that mode code around, reallocate stack space, etc. that cause the undefined variable to have different (garbage) values. What might happen to work one day most assuredly will not when compiled with a compiler 2-3 versions newer.

A good example of this can be seen in GCC's constant propagation implem (tree-ssa-ccp.c):

/*
...
- If an argument has an UNDEFINED value, then it does not affect
  the outcome of the meet operation.  If a variable V_i has an
  UNDEFINED value, it means that either its defining statement
  hasn't been visited yet or V_i has no defining statement, in
  which case the original symbol 'V' is being used
  uninitialized. Since 'V' is a local variable, the compiler
  may assume any initial value for it.
...
*/

And we can see this in action with e.g.

size_t f(int x){size_t a; if(x) a = 42; return a;}

which gives

f:
  movl  $42, %eax
  ret
  .ident  "GCC: (GNU) 4.8.0 20120408 (experimental)"

ACCESS_ONCE()

daglwn — Wed, 08 Aug 2012 16:56:10 +0000

> It's not the code that it's harmful, but the transformation the compiler
> does in the optimization.

No. The code is the specification of what the programmer wants to happen. If the programmer doesn't mark something "volatile" when it should be, there's no way the compiler can automatically infer that information.

> Because there's no way it can be sure if the transformation is safe (or
> not!), it should not be doing it.

You've just killed all compiler optimization.

> Isn't it common sense that any code compiled today _can_ be used in a
> multi-threaded environment?

And any variable might be shared. So now the compiler must assume everything is volatile.

You've just killed all compiler optimization.

ACCESS_ONCE()

daglwn — Wed, 08 Aug 2012 16:51:47 +0000

That *is* the --do-what-I-mean-not-what-I-write switch.

How can the compiler possibly know what you meant when you wrote something outside the language spec?

Take a very simple but common issue: undefined data. There are countless compiler analyses and transformations that mode code around, reallocate stack space, etc. that cause the undefined variable to have different (garbage) values. What might happen to work one day most assuredly will not when compiled with a compiler 2-3 versions newer.

What should the compiler do? Disable all code motion of and around the offending expression? That would be lunacy.

Declaring it volatile

PaulMcKenney — Wed, 08 Aug 2012 15:26:45 +0000

To each their own, I guess.

ACCESS_ONCE()

dgm — Wed, 08 Aug 2012 14:30:29 +0000

> The problem is that the compiler has *no way* to know if the code is potentially harmful

It's not the code that it's harmful, but the transformation the compiler does in the optimization. To put it another way: the compiler chooses to do something that is NOT what the code says. It does so because it assumes it's safe. Because there's no way it can be sure if the transformation is safe (or not!), it should not be doing it.

> The compiler doesn't know which parts of code will be operating in a multi-threaded environment

Isn't it common sense that any code compiled today _can_ be used in a multi-threaded environment?

ACCESS_ONCE()

dgm — Wed, 08 Aug 2012 13:31:01 +0000

> The famous --do-what-I-mean-not-what-I-write compiler switch.

That would be a good switch, yes. By default the compiler should be in --do-what-I-write-not-what-some-ambiguous-line-of-the-spec-allows-you-to-interpret, though. It would be better for everybody.

Declaring it volatile

PaulMcKenney — Tue, 07 Aug 2012 21:26:33 +0000

Here is the URL you are looking for: http://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

Declaring it volatile

paulj — Tue, 07 Aug 2012 21:12:31 +0000

I have to ask, what is this ({…}) GCCism? I've searched the manual and online for it and "variably modified type" but not found anything useful.

Declaring it volatile

daglwn — Tue, 07 Aug 2012 19:44:50 +0000

If the programmmer is optimizing by using a temporary, the assumption is that the programmer knows what he or she is doing. The optimization is explicit and is an immediate flag to the reader to carefully examine the (existing or new) code for bugs.

I'm not trying to convince you, simply stating what I believe is good programming practice. Maybe I'm wrong. But it's worked for me so far. :)

ACCESS_ONCE()

daglwn — Tue, 07 Aug 2012 19:42:16 +0000

Oh, we have plenty of cases where we try to cater to users who stretch (to put it mildly) the standard. But one must recognize that the programmer is no longer programming in said language. He or she is programming in some other, ill-defined language.

My point is that a bug caused by the programmer not adhering to the language standards is not caused by the compiler. The compiler team may be willing to work around the problem but that doesn't mean the code ain't broke.

> If something has the potential of doing harm, and the user is perfectly
> capable of doing it by hand anyway if really needed, then it's best for
> the compiler not to do it.

The problem is that the compiler has *no way* to know if the code is potentially harmful because the code is doing something outside the definition of the language. The problem ACCESS_ONCE is trying to solve is a perfect example. The compiler doesn't know which parts of code will be operating in a multi-threaded environment, much less which pieces of memory are shared. We have "volatile" to mark variables in a way that happens to work for our current threading models. But it is the only thing we have in C right now.

Compiler writers generally don't want to limit legal transformations because those transformations can help the vast majority of programmers who don't have whatever specialized problem one programmer might complain about.