Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Linus Torvalds
Date: Thursday, August 16, 2007 - 8:42 pm

On Fri, 17 Aug 2007, Paul Mackerras wrote:

One of the things that "volatile" generally screws up is a simple

	volatile int i;

	i++;

which a compiler will generally get horribly, horribly wrong.

In a reasonable world, gcc should just make that be (on x86)

	addl $1,i(%rip)

on x86-64, which is indeed what it does without the volatile. But with the 
volatile, the compiler gets really nervous, and doesn't dare do it in one 
instruction, and thus generates crap like

        movl    i(%rip), %eax
        addl    $1, %eax
        movl    %eax, i(%rip)

instead. For no good reason, except that "volatile" just doesn't have any 
good/clear semantics for the compiler, so most compilers will just make it 
be "I will not touch this access in any way, shape, or form". Including 
even trivially correct instruction optimization/combination.

This is one of the reasons why we should never use "volatile". It 
pessimises code generation for no good reason - just because compilers 
don't know what the heck it even means! 

Now, people don't do "i++" on atomics (you'd use "atomic_inc()" for that), 
but people *do* do things like

	if (atomic_read(..) <= 1)
		..

On ppc, things like that probably don't much matter. But on x86, it makes 
a *huge* difference whether you do

	movl i(%rip),%eax
	cmpl $1,%eax

or if you can just use the value directly for the operation, like this:

	cmpl $1,i(%rip)

which is again a totally obvious and totally safe optimization, but is 
(again) something that gcc doesn't dare do, since "i" is volatile.

In other words: "volatile" is a horribly horribly bad way of doing things, 
because it generates *worse*code*, for no good reason. You just don't see 
it on powerpc, because it's already a load-store architecture, so there is 
no "good code" for doing direct-to-memory operations.

		Linus
-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Christoph Lameter, (Tue Aug 14, 4:14 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Wed Aug 15, 11:31 am)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Paul E. McKenney, (Wed Aug 15, 11:57 am)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Wed Aug 15, 1:47 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Wed Aug 15, 1:52 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Wed Aug 15, 2:05 pm)
[No subject], Satyam Sharma, (Wed Aug 15, 5:36 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Wed Aug 15, 6:23 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Wed Aug 15, 6:26 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Wed Aug 15, 6:30 pm)
Re: , Segher Boessenkool, (Wed Aug 15, 6:38 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Thu Aug 16, 12:32 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Thu Aug 16, 12:33 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Linus Torvalds, (Thu Aug 16, 8:42 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Paul E. McKenney, (Thu Aug 16, 10:18 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Geert Uytterhoeven, (Thu Aug 16, 11:42 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Fri Aug 17, 10:37 am)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Fri Aug 17, 3:29 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Fri Aug 17, 3:49 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Fri Aug 17, 4:55 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Christoph Lameter, (Fri Aug 17, 6:24 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Mon Aug 20, 3:04 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Mon Aug 20, 4:02 pm)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Tue Aug 21, 7:39 am)
Re: [PATCH 0/24] make atomic_read() behave consistently ac ..., Segher Boessenkool, (Tue Aug 21, 7:48 am)
Re: [PATCH] i386: Fix a couple busy loops in mach_wakecpu. ..., Christoph Lameter, (Fri Aug 24, 10:06 am)