On Wed, Jan 21, 2015 at 05:39:12PM +0000, Alex Velenko wrote:
> Hi,
> Is the following patch ok?
> regards,
> Alex
Hi Alex,
Some comments on your submission inline below.
> This patch fixes aarch64/atomic-op-consume.c test to expect safe assembly to
> be
> generated when __ATOMIC_CONSUME semantics is requested.
This text should provide the context for the fix. In particular, it
should explain what is meant by "safe" assembly. In this case, the reason
for your fix is included in the patch as a comment, but you need to give
enough information up top so that we can understand what your patch is
about and why it is correct.
> 2015-01-21 Alex Velenko alex.vele...@arm.com
>
> gcc/testsuite/
>
> gcc.target/aarch64/atomic-op-consume.c(scan-assember-times): Modified.
This ChangeLog entry is not of the correct format, I would have expected
something like:
2015-01-21 Alex Velenko <alex.vele...@arm.com>
* gcc.target/aarch64/atomic-op-consume.c
(scan-assembler-times): Adjust scan-assembler-times pattern.
Note: Two spaces between the date and your name, two spaces between your
name and your email address, "< >" around your email address, * before
the filepath.
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/atomic-op-consume.c
> b/gcc/testsuite/gcc.target/aarch64/atomic-op-consume.c
> index 38d6c2c..cf33be2 100644
> --- a/gcc/testsuite/gcc.target/aarch64/atomic-op-consume.c
> +++ b/gcc/testsuite/gcc.target/aarch64/atomic-op-consume.c
> @@ -3,5 +3,9 @@
>
> #include "atomic-op-consume.x"
>
> -/* { dg-final { scan-assembler-times "ldxr\tw\[0-9\]+, \\\[x\[0-9\]+\\\]" 6
> } } */
> +/* To workaround Bugzilla 59448 issue, a request for memory behaviour
> + __ATOMIC_CONSUME is promoted to MEMMODEL_ACQUIRE behaviour, not
> + MEMMODEL_CONSUME behaviour. This causes "ldaxr" to be generated
> + instead of "ldxr". */
This text confuses some terminology and mixes internal implementation
details and external predefine names.
In the interests of being accurate (we might forget the context of
this fix in future), I would suggest the following:
To workaround Bugzilla 59448, a request for __ATOMIC_CONSUME is always
promoted to __ATOMIC_ACQUIRE. This causes "LDAXR" to be generated
instead of "LDXR".
Here, we drop the internal implementation details of the compiler
(which may change and end up out of sync with this testcase) and concentrate
only on the user visible behaviours.
Thanks,
James
