[PATCH] Handle target specific memory models in C frontend

[PATCH] Handle target specific memory models in C frontend

[Andi Kleen]

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(-)

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,
-- 
1.7.11.5

Re: [PATCH] Handle target specific memory models in C frontend

[Richard Henderson]

On 2012-11-09 07:03, Andi Kleen wrote:
> 	PR55139
> 	* c-common.c (get_atomic_generic_size): Mask with
>         MEMMODEL_MASK

Ok.


r~

Re: [PATCH] Handle target specific memory models in C frontend

[Jakub Jelinek]

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

Re: [PATCH] Handle target specific memory models in C frontend

[Richard Henderson]

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~

Re: [PATCH] Handle target specific memory models in C frontend

[Andi Kleen]

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

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH] Handle target specific memory models in C frontend

[Andi Kleen]

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


-- 
ak@linux.intel.com -- Speaking for myself only.

Re: *PING*BACKPORT* [PATCH] Handle target specific memory models in C frontend

[Andi Kleen]

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?

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH] Handle target specific memory models in C frontend

[Richard Henderson]

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~