| Message ID | 1352473427-11967-1-git-send-email-andi@firstfloor.org |
|---|---|
| State | New |
| Headers | show |
On 2012-11-09 07:03, Andi Kleen wrote: > PR55139 > * c-common.c (get_atomic_generic_size): Mask with > MEMMODEL_MASK Ok. r~
Hi! Just nits. On Fri, Nov 09, 2012 at 04:03:47PM +0100, Andi Kleen wrote: > From: Andi Kleen <ak@linux.intel.com> > > get_atomic_generic_size would error out for > __atomic_store(...,__ATOMIC_HLE_RELEASE) > > Just mask it out. All the memory orders are checked completely > in builtins.c anyways. > > I'm not sure what that check is for, it could be removed in theory. I think it actually should be removed, given that e.g. int i, j; void foo (void) { __atomic_store (&i, &j, 12); } warns twice for the same thing, when it should only warn once. Richard? > Passed bootstrap and test suite on x86-64 > > gcc/c-family/: > 2012-11-09 Andi Kleen <ak@linux.intel.com> > > PR55139 That should be PR target/55139 > * c-common.c (get_atomic_generic_size): Mask with > MEMMODEL_MASK Dot after MEMMODEL_MASK and don't wrap it on next line. > --- > gcc/c-family/c-common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c > index c3a6465..7828d21 100644 > --- a/gcc/c-family/c-common.c > +++ b/gcc/c-family/c-common.c > @@ -10097,7 +10097,7 @@ get_atomic_generic_size (location_t loc, tree function, VEC(tree,gc) *params) > if (TREE_CODE (p) == INTEGER_CST) > { > int i = tree_low_cst (p, 1); > - if (i < 0 || i >= MEMMODEL_LAST) > + if (i < 0 || (i & MEMMODEL_MASK) >= MEMMODEL_LAST) > { > warning_at (loc, OPT_Winvalid_memory_model, > "invalid memory model argument %d of %qE", x + 1, If that whole loop doesn't get removed, there are some formatting issues in it: for (x = n_param - n_model ; x < n_param; x++) shouldn't have space before ; and error_at (loc, "non-integer memory model argument %d of %qE", x + 1, function); doesn't correctly horizontally aling the second line. Jakub
On 2012-11-09 07:18, Jakub Jelinek wrote: > I think it actually should be removed, given that e.g. > int i, j; > void > foo (void) > { > __atomic_store (&i, &j, 12); > } > warns twice for the same thing, when it should only warn once. > Richard? Makes sense. r~
Richard Henderson <rth@redhat.com> writes: > On 2012-11-09 07:18, Jakub Jelinek wrote: >> I think it actually should be removed, given that e.g. >> int i, j; >> void >> foo (void) >> { >> __atomic_store (&i, &j, 12); >> } >> warns twice for the same thing, when it should only warn once. >> Richard? > > Makes sense. Ok, I'll remove it completely and fix the ChangeLog. Thanks. -Andi
On Fri, Nov 09, 2012 at 07:08:07AM -0800, Richard Henderson wrote: > On 2012-11-09 07:03, Andi Kleen wrote: > > PR55139 > > * c-common.c (get_atomic_generic_size): Mask with > > MEMMODEL_MASK > > Ok. I would like to backport this patch to 4.8. Ok? -Andi
Andi Kleen <andi@firstfloor.org> writes: > On Fri, Nov 09, 2012 at 07:08:07AM -0800, Richard Henderson wrote: >> On 2012-11-09 07:03, Andi Kleen wrote: >> > PR55139 >> > * c-common.c (get_atomic_generic_size): Mask with >> > MEMMODEL_MASK >> >> Ok. > > I would like to backport this patch to 4.8. Ok? > Ping?
On 08/10/2013 12:40 PM, Andi Kleen wrote: > On Fri, Nov 09, 2012 at 07:08:07AM -0800, Richard Henderson wrote: >> On 2012-11-09 07:03, Andi Kleen wrote: >>> PR55139 >>> * c-common.c (get_atomic_generic_size): Mask with >>> MEMMODEL_MASK >> >> Ok. > > I would like to backport this patch to 4.8. Ok? Ok. r~
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index c3a6465..7828d21 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -10097,7 +10097,7 @@ get_atomic_generic_size (location_t loc, tree function, VEC(tree,gc) *params) if (TREE_CODE (p) == INTEGER_CST) { int i = tree_low_cst (p, 1); - if (i < 0 || i >= MEMMODEL_LAST) + if (i < 0 || (i & MEMMODEL_MASK) >= MEMMODEL_LAST) { warning_at (loc, OPT_Winvalid_memory_model, "invalid memory model argument %d of %qE", x + 1,
From: Andi Kleen <ak@linux.intel.com> get_atomic_generic_size would error out for __atomic_store(...,__ATOMIC_HLE_RELEASE) Just mask it out. All the memory orders are checked completely in builtins.c anyways. I'm not sure what that check is for, it could be removed in theory. Passed bootstrap and test suite on x86-64 gcc/c-family/: 2012-11-09 Andi Kleen <ak@linux.intel.com> PR55139 * c-common.c (get_atomic_generic_size): Mask with MEMMODEL_MASK --- gcc/c-family/c-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)