From: "Joseph S. Myers" <joseph@codesourcery.com>
To: Dinar Temirbulatov <dtemirbulatov@gmail.com>
Cc: <libc-ports@sourceware.org>, <adeb@nvidia.com>
Subject: Re: [Patch] ARM define atomic_exchange_acq/atomic_exchange_rel to __atomic_exchange_n
Date: Mon, 02 Sep 2013 15:08:00 -0000 [thread overview]
Message-ID: <Pine.LNX.4.64.1309021457010.17654@digraph.polyomino.org.uk> (raw)
In-Reply-To: <CAMnfPmMADb=ZqSmC4J6=fMpQp56uGG8v9MKRj_Ly94kcXEtNZA@mail.gmail.com>
On Mon, 2 Sep 2013, Dinar Temirbulatov wrote:
> Hi,
> Another version of change, I added
> atomic_compare_and_exchange_val_acq/atomic_compare_and_exchange_val_rel
> and atomic_compare_and_exchange_bool_acq/atomic_compare_and_exchange_boot_rel
> defenitions and also for gcc-4.7 and higher in the case of unsupported
> atomic compare and swap operation, it uses the kernel helper inlines.
> Tested on arm a9 with no new regressions. Ok to commit?
> Oh, sorry. I missed to attach the change. Here it is.
For subsequent patch revisions, please note there should be an extra space
after "#" for preprocessor directives inside #if conditionals, one per
level of #if nesting (other than toplevel multiple-inclusion guards) -
which means that if conditioning existing code, you need to adjust
directives inside that code.
This patch appears to have too much duplication. For example, you
duplicate the definition of __arm_assisted_compare_and_exchange_val_32_acq
- but that should not need any extra conditionals at all (beyond the
existing #ifndef __arm_assisted_compare_and_exchange_val_32_acq), there's
no reason ever not to define it. Similarly, you duplicate
__arch_compare_and_exchange_val_64_acq, but with proper #if structure
there should only need to be one copy of the version that uses
__arm_link_error.
What I think you should aim for is that each definition, or small group of
definitions, uses conditionals in the form
#if __GNUC_PREREQ (4, 7) && defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
/* Version using __atomic_*. */
#elif defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
/* Version using __sync_*. */
#else
/* Version using __arm_assisted_*. */
#endif
with cases omitted if not useful for that particular macro (this may
include some macros not being defined at all in some cases). So don't
insert any global conditionals affecting all the existing definitions at
all - look at each block of conditionals and add a third case as needed,
along with any new macros (again with conditionals in that form) that are
appropriate.
Where you use abort () in some definitions, use __arm_link_error ()
instead, like for the existing definitions.
--
Joseph S. Myers
joseph@codesourcery.com
next prev parent reply other threads:[~2013-09-02 15:08 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-01 20:43 Dinar Temirbulatov
2013-09-02 15:08 ` Joseph S. Myers [this message]
2013-09-03 17:31 ` Abhishek Deb
2013-09-05 12:56 ` Dinar Temirbulatov
2013-09-05 15:48 ` Joseph S. Myers
2013-09-05 17:09 ` Abhishek Deb
2013-09-11 19:25 ` Dinar Temirbulatov
2013-09-11 19:27 ` Dinar Temirbulatov
2013-09-11 20:40 ` Abhishek Deb
2013-09-13 17:14 ` Joseph S. Myers
2013-09-14 3:29 ` Dinar Temirbulatov
2013-09-18 17:16 ` Joseph S. Myers
2013-09-19 6:53 ` Maxim Kuvyrkov
2013-09-19 16:29 ` Joseph S. Myers
-- strict thread matches above, loose matches on Subject: below --
2013-09-01 20:42 Dinar Temirbulatov
2013-08-13 17:11 Dinar Temirbulatov
2013-08-15 21:46 ` Abhishek Deb
2013-08-16 22:57 ` Dinar Temirbulatov
2013-08-17 2:42 ` Abhishek Deb
2013-08-19 19:21 ` Dinar Temirbulatov
2013-08-18 20:29 ` Joseph S. Myers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.64.1309021457010.17654@digraph.polyomino.org.uk \
--to=joseph@codesourcery.com \
--cc=adeb@nvidia.com \
--cc=dtemirbulatov@gmail.com \
--cc=libc-ports@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).