Handle target specific memory models in C frontend

Submitted by Andi Kleen on Nov. 9, 2012, 3:03 p.m.

Details

Message ID 1352473427-11967-1-git-send-email-andi@firstfloor.org
State New
Headers show

Commit Message

Andi Kleen Nov. 9, 2012, 3:03 p.m.
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(-)

Comments

Richard Henderson Nov. 9, 2012, 3:08 p.m.
On 2012-11-09 07:03, Andi Kleen wrote:
> 	PR55139
> 	* c-common.c (get_atomic_generic_size): Mask with
>         MEMMODEL_MASK

Ok.


r~
Jakub Jelinek Nov. 9, 2012, 3:18 p.m.
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
Richard Henderson Nov. 9, 2012, 4:43 p.m.
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~
Andi Kleen Nov. 9, 2012, 4:54 p.m.
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
Andi Kleen Aug. 10, 2013, 7:40 p.m.
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 Sept. 8, 2013, 7:41 p.m.
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?
Richard Henderson Sept. 9, 2013, 6:33 p.m.
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~

Patch hide | download patch | download mbox

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,