| Message ID | 1421861952-6366-1-git-send-email-alex.velenko@arm.com |
|---|---|
| State | New |
| Headers | show |
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.Velenko@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.Velenko@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
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". */ +/* { dg-final { scan-assembler-times "ldaxr\tw\[0-9\]+, \\\[x\[0-9\]+\\\]" 6 } } */ /* { dg-final { scan-assembler-times "stxr\tw\[0-9\]+, w\[0-9\]+, \\\[x\[0-9\]+\\\]" 6 } } */
Hi, Is the following patch ok? regards, Alex This patch fixes aarch64/atomic-op-consume.c test to expect safe assembly to be generated when __ATOMIC_CONSUME semantics is requested. 2015-01-21 Alex Velenko Alex.Velenko@arm.com gcc/testsuite/ gcc.target/aarch64/atomic-op-consume.c(scan-assember-times): Modified.