Andrew is right.
Volatile makes compiler read variable value from memory each time.
Reading from memory does not mean that it will skip the cache. It
still could get the value from the cache if cache is enabled and
valid. Volatile and cache are different stores.
Thanks!
Jeff
*From:*Andrew Fish [mailto:af...@apple.com]
*Sent:* Thursday, March 05, 2015 10:31 AM
*To:* edk2-devel@lists.sourceforge.net
*Cc:* Fan, Jeff
*Subject:* Re: [edk2] [v2 4/4] UefiCpuPkg/MpSerivce: remove volatile
qualifier
On Mar 4, 2015, at 5:34 PM, Chen Fan <chen.fan.f...@cn.fujitsu.com
<mailto:chen.fan.f...@cn.fujitsu.com>> wrote:
Hi Jeff,
Thanks for your explanation, I didn't encounter any issue if
leaving it
there.
I just think if use Volatile qualifier may cause the problem of
performance. because
access the value need to copy the address value from main memory
to cpu
cache
each time and not use the cache.
and IIRC, in a shared memory multiprocessor system, the cache
coherence may
keep the consistency of shared resource data in each processor local
caches.
if the value is updated by one processor in local cache, the copy of
memory data in
other processor caches will be marked as invalid. and then other
processors read the
value will access from the main memory.
if I misunderstand, please correct me. :)
Correction!
Volatile in C has nothing to do with hardware and caching. It has to
do with the abstract memory model for the compiler as defined by the C
standard. The more aggressive the compiler optimizes the code the more
you are exposed to potential issues. If the compiler knows about all
the code in the program that accesses a variable it can assume that
memory location will only be changed by the compiler, and make
optimizations (like storing a pointer in a register).
A common optimization is to avoid memory reads that are not needed.
Here is a simple example in clang, probably the most aggressive
optimizing compiler I know of…. (My comments start with #):
~/work/Compiler>cat volatile.c
int
main (int argc, char **argv)
{
int *i = (int *)0xFFFFFFF0;
while (*i != 0);
return 0;
}
~/work/Compiler>clang volatile.c -S -O3
~/work/Compiler>cat volatile.S
.section__TEXT,__text,regular,pure_instructions
.globl_main
.align4, 0x90
_main: ## @main
pushq%rbp
movq%rsp, %rbp
movl$4294967280, %eax ## imm = 0xFFFFFFF0
cmpl$0, (%rax)
jeLBB0_2
# Notice the compiler only does a single read. The memory model of C
implies this memory will not change.
.align4, 0x90
LBB0_1: ## %..split_crit_edge
## =>This Inner Loop Header: Depth=1
jmpLBB0_1
# Notice this while loop is just an infinite loop? There is no point
reading the memory since the C spec states it will not change.
LBB0_2: ## %.split1
xorl%eax, %eax
popq%rbp
retq
If I make i a volatile variable it forces the compiler to read it
every time: volatile int *i = (int *)0xFFFFFFF0;
~/work/Compiler>cat volatile.S
.section__TEXT,__text,regular,pure_instructions
.globl_main
.align4, 0x90
_main: ## @main
pushq%rbp
movq%rsp, %rbp
movl$4294967280, %eax ## imm = 0xFFFFFFF0
.align4, 0x90
LBB0_1: ## =>This Inner Loop Header: Depth=1
cmpl$0, (%rax)
jneLBB0_1
# Now it reads every iteration of the while loop
xorl%eax, %eax
popq%rbp
retq
So the point of making it volatile is to force the compiler to read
the value every time the C code asks for the value.
Thanks,
Andrew Fish
Thanks,
Chen
On 03/04/2015 03:56 PM, Fan, Jeff wrote:
Chen,
Volatile is to tell compiler this variable maybe changed by other
threads. Compiler must access the value from memory instead of the
old value in its registers. This is different with spinlock that is
to protect shared memory between multiple threads.
For this case, CpuState may be updated by BSP/AP. I prefer to keep
volatile type. Do you meet any issue if keepping this type?
Besides it, I also suggest to add volatile for the following variable.
CpuData->Parameter
CpuData->Procedure
Thanks!
Jeff
-----Original Message-----
From: Chen Fan [mailto:chen.fan.f...@cn.fujitsu.com]
Sent: Tuesday, March 03, 2015 1:06 PM
To: edk2-devel@lists.sourceforge.net
<mailto:edk2-devel@lists.sourceforge.net>
Cc: Fan, Jeff; Justen, Jordan L
Subject: [v2 4/4] UefiCpuPkg/MpSerivce: remove volatile qualifier
because manipulating CpuState always with lock protection. so volatile
qualifier is redundant.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com
<mailto:chen.fan.f...@cn.fujitsu.com>>
---
UefiCpuPkg/CpuDxe/CpuMp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuMp.h b/UefiCpuPkg/CpuDxe/CpuMp.h
index cb3460f..e54b571 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.h
+++ b/UefiCpuPkg/CpuDxe/CpuMp.h
@@ -92,7 +92,7 @@ typedef struct {
EFI_PROCESSOR_INFORMATION Info;
SPIN_LOCK CpuDataLock;
INTN LockSelf;
- volatile CPU_STATE State;
+ CPU_STATE State;
EFI_AP_PROCEDURE Procedure;
VOID *Parameter;
--
1.9.3
.
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website,
sponsored
by Intel and developed in partnership with Slashdot Media, is your hub
for all
things parallel software development, from weekly thought leadership
blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net <mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
