Thread (49 messages) 49 messages, 8 authors, 2021-07-30

Re: [dpdk-dev] [PATCH v2 0/8] use GCC's C11 atomic builtins for test

From: Joyce Kong <hidden>
Date: 2021-07-13 07:28:36

Hi David,

Since we have some discussion about the atomic operations now, I changed the commit message from "C11 atomics"(which has been widely used in previous commit) to "GCC atomic built-ins".
What's your opinion about whether keeping the previous message for the consistency or using the new description?

Joyce
-----Original Message-----
From: Honnappa Nagarahalli <redacted>
Sent: Thursday, July 1, 2021 6:41 AM
To: Tyler Retzlaff <redacted>
Cc: thomas@monjalon.net; Joyce Kong <redacted>;
dev@dpdk.org; david.marchand@redhat.com;
stephen@networkplumber.org; olivier.matz@6wind.com;
andrew.rybchenko@oktetlabs.ru; harry.van.haaren@intel.com; Ruifeng
Wang [off-list ref]; nd [off-list ref]; techboard@dpdk.org;
nd [off-list ref]
Subject: RE: [dpdk-dev] [PATCH v2 0/8] use GCC's C11 atomic builtins for test

<snip>
quoted
quoted
quoted
quoted
As I mentioned earlier in this thread, GCC supports 2 types of
atomics. "Use GCC atomic builtins" does not help distinguish
between them. In "GCC's C11 atomic builtins" - "C11" indicates
which atomics we are using, "atomic builtins" indicates that we
are NOT using APIs from stdatomic.h
if you need a term to distinguish the two sets of atomics in gcc
you can qualify it with "Memory Model Aware" which is straight
from the gcc
manual.
quoted
"Memory model aware" sounds too generic. The same page [1] also
makes
it clear that the built-in functions match the requirements for the
C11 memory model.

allow me to put your interpretation of the manual that you linked side
by side with what the manual text actually says verbatim.

your text from above
  "built-in functions match the requirements for the C11 memory model."

the actual text from your link
  "built-in functions approximately match the requirements for the
C++11 memory model."

* you've chosen to drop approximately from the wording to try and make
  your argument.
I am not sure how this makes a difference to our arguments. For ex: there
are no other built in functions that "exactly" match the C++11 memory model
supported by GCC.
quoted
* you've also chosen to substitute C11 in place of C++11. again
  presumably for the same reason.

in fact the entire page does not mention C11 even once, it also goes
on to highlight a specific deviation from C++11 with this excerpt
"because of a deficiency in C++11's semantics for
memory_order_consume"
I do not have a problem to call it C++11. IMO, calling it "GCC's C++11 ..." will
address this deviation and the approximation.
quoted
quoted
There are also several patches merged in the past which do not use
the term
"memory model aware". I would prefer to be consistent.

i prefer the history represent the change. that previous submitters
and reviewers lacked precision is not my concern nor is consistency a
reason to continue documenting history incorrectly.
Ok. As I mentioned, it is just my preference.
quoted
i'm waiting to ack the change, it's up to you. you've already spent
more time arguing than it would have taken to submit a v2 correcting the
problem.
I am not arguing for the sake of arguing. You are trying to correct few
mistakes here (I truly appreciate that) and I am trying to explain my POV and
making corrections as needed. I am sure we will conclude soon.
quoted
quoted
[1]
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help