public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug other/97309] New: Improve documentation of -fallow-store-data-races
@ 2020-10-06 18:54 qinzhao at gcc dot gnu.org
2020-10-06 20:39 ` [Bug other/97309] " qinzhao at gcc dot gnu.org
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2020-10-06 18:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97309
Bug ID: 97309
Summary: Improve documentation of -fallow-store-data-races
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: other
Assignee: unassigned at gcc dot gnu.org
Reporter: qinzhao at gcc dot gnu.org
Target Milestone: ---
As of GCC 10, the former --param allow-store-data-races is now
-fallow-store-data-races. The default, in both cases, is not to allow them.
For releases prior to GCC 10, the --param is documented as
allow-store-data-races
Allow optimizers to introduce new data races on stores. Set
to 1 to allow, otherwise to 0.
The description for GCC 10 is simply:
-fallow-store-data-races
Allow the compiler to introduce new data races on stores.
Enabled at level -Ofast.
There are three problems with this description.
(1) The explanation is sparse - basically it just repeats the name of the
switch.
(2) It provides no context to explain the circumstances under which it may or
may not be safe to use.
(3) Because of the lack of clarity regarding safety, it may be questionable
as to whether a SPEC CPU user is allowed to use -Ofast
(http://www.spec.org/cpu2017/Docs/runrules.html#safe)
Suggested improvement:
Allow the compiler to perform optimizations that may introduce new data races
on stores, without proving that the variable cannot be concurrently accessed
by other threads. Does not affect optimization of local data. It is safe to
use this option if it is known that global data will not be accessed by
multiple threads.
Examples of optimizations enabled by -fallow-store-data-races include
hoisting or if-conversions that may cause a value that was already in memory
to be re-written with that same value. Such re-writing is safe in a single
threaded context but may be unsafe in a multi-threaded context. Note that on
some processors, if-conversions may be required in order to enable
vectorization.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug other/97309] Improve documentation of -fallow-store-data-races
2020-10-06 18:54 [Bug other/97309] New: Improve documentation of -fallow-store-data-races qinzhao at gcc dot gnu.org
@ 2020-10-06 20:39 ` qinzhao at gcc dot gnu.org
2020-10-08 15:01 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2020-10-06 20:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97309
--- Comment #1 from qinzhao at gcc dot gnu.org ---
proposed patch:
Subject: [PATCH] PR97309--improve documentation of -fallow-store-data-races
---
gcc/doc/invoke.texi | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 7c81d7f41bd..926ee1ff28e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11621,7 +11621,18 @@ Do not remove unused C++ allocations in dead code
elimination.
@item -fallow-store-data-races
@opindex fallow-store-data-races
-Allow the compiler to introduce new data races on stores.
+Allow the compiler to perform optimizations that may introduce new data races
+on stores, without proving that the variable cannot be concurrently accessed
+by other threads. Does not affect optimization of local data. It is safe to
+use this option if it is known that global data will not be accessed by
+multiple threads.
+
+Examples of optimizations enabled by @option{-fallow-store-data-races} include
+hoisting or if-conversions that may cause a value that was already in memory
+to be re-written with that same value. Such re-writing is safe in a single
+threaded context but may be unsafe in a multi-threaded context. Note that on
+some processors, if-conversions may be required in order to enable
+vectorization.
Enabled at level @option{-Ofast}.
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug other/97309] Improve documentation of -fallow-store-data-races
2020-10-06 18:54 [Bug other/97309] New: Improve documentation of -fallow-store-data-races qinzhao at gcc dot gnu.org
2020-10-06 20:39 ` [Bug other/97309] " qinzhao at gcc dot gnu.org
@ 2020-10-08 15:01 ` cvs-commit at gcc dot gnu.org
2020-10-08 15:03 ` qinzhao at gcc dot gnu.org
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-10-08 15:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97309
--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Qing Zhao <qinzhao@gcc.gnu.org>:
https://gcc.gnu.org/g:baf4750feaa6a5fa502ae7bc0b90f31640af6f47
commit r11-3732-gbaf4750feaa6a5fa502ae7bc0b90f31640af6f47
Author: qing zhao <qinzhao@gcc.gnu.org>
Date: Thu Oct 8 17:01:07 2020 +0200
Improve documentation of -fallow-store-data-races
2020-10-08 John Henning <john.henning@oracle.com>
gcc/
PR other/97309
* doc/invoke.texi: Improve documentation of
-fallow-store-data-races.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug other/97309] Improve documentation of -fallow-store-data-races
2020-10-06 18:54 [Bug other/97309] New: Improve documentation of -fallow-store-data-races qinzhao at gcc dot gnu.org
2020-10-06 20:39 ` [Bug other/97309] " qinzhao at gcc dot gnu.org
2020-10-08 15:01 ` cvs-commit at gcc dot gnu.org
@ 2020-10-08 15:03 ` qinzhao at gcc dot gnu.org
2022-05-12 8:55 ` redi at gcc dot gnu.org
2022-05-12 8:58 ` redi at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2020-10-08 15:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97309
qinzhao at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
--- Comment #3 from qinzhao at gcc dot gnu.org ---
fixed.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug other/97309] Improve documentation of -fallow-store-data-races
2020-10-06 18:54 [Bug other/97309] New: Improve documentation of -fallow-store-data-races qinzhao at gcc dot gnu.org
` (2 preceding siblings ...)
2020-10-08 15:03 ` qinzhao at gcc dot gnu.org
@ 2022-05-12 8:55 ` redi at gcc dot gnu.org
2022-05-12 8:58 ` redi at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: redi at gcc dot gnu.org @ 2022-05-12 8:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97309
--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think this is still unclear.
What does "new data races" mean? Can it introduce races in code that had none
previously? Or only add new ones to code that already has them?
Does this make -Ofast incompatible with multithreaded programs?
Does this only apply to non-atomic loads and stores?
If all accesses to a variable use atomic ops, does that mean it's immune from
the unsafe optimizations enabled by this flag? If no, that makes -Ofast
unusable in MT code. If yes, good, but why is the flag needed? If there are
non-atomic accesses to a variable, we can already assume it's not concurrently
accessed, because any such potentially concurrent conflicting action would
already be a data race and UB. If we already have data races with UB, can't we
just introduce more? Is this flag saying "allow the compiler to make existing
UB even worse"? If not, what is it saying?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug other/97309] Improve documentation of -fallow-store-data-races
2020-10-06 18:54 [Bug other/97309] New: Improve documentation of -fallow-store-data-races qinzhao at gcc dot gnu.org
` (3 preceding siblings ...)
2022-05-12 8:55 ` redi at gcc dot gnu.org
@ 2022-05-12 8:58 ` redi at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: redi at gcc dot gnu.org @ 2022-05-12 8:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97309
--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #4)
> If all accesses to a variable use atomic ops, does that mean it's immune
> from the unsafe optimizations enabled by this flag? If no, that makes -Ofast
> unusable in MT code. If yes, good, but why is the flag needed? If there are
> non-atomic accesses to a variable, we can already assume it's not
> concurrently accessed, because any such potentially concurrent conflicting
> action would already be a data race and UB. If we already have data races
> with UB, can't we just introduce more? Is this flag saying "allow the
> compiler to make existing UB even worse"? If not, what is it saying?
Or maybe this flag is relevant for languages that don't use the C and C++
memory models, where the rules are different?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-12 8:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-06 18:54 [Bug other/97309] New: Improve documentation of -fallow-store-data-races qinzhao at gcc dot gnu.org
2020-10-06 20:39 ` [Bug other/97309] " qinzhao at gcc dot gnu.org
2020-10-08 15:01 ` cvs-commit at gcc dot gnu.org
2020-10-08 15:03 ` qinzhao at gcc dot gnu.org
2022-05-12 8:55 ` redi at gcc dot gnu.org
2022-05-12 8:58 ` redi at gcc dot gnu.org
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 read-only IMAP folder(s) and NNTP newsgroup(s).