[Patch][OpenCL] Custom atomic Builtin check ignores address space of a non-atomic pointer

[Anastasia Stulova via cfe-commits]

Could someone jump in, please!

I have made a small improvement in testing after the patch has been first approved by Pekka.

The last change is essentially this:

Index: test/CodeGen/atomic-ops.c
===================================================================
--- test/CodeGen/atomic-ops.c	(revision 250025)
+++ test/CodeGen/atomic-ops.c	(working copy)
@@ -1,10 +1,10 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -ffreestanding -triple=i686-apple-darwin9 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -ffreestanding -ffake-address-space-map -triple=i686-apple-darwin9 | FileCheck %s
 // REQUIRES: x86-registered-target
 
 // Also test serialization of atomic operations here, to avoid duplicating the
 // test.
-// RUN: %clang_cc1 %s -emit-pch -o %t -ffreestanding -triple=i686-apple-darwin9
-// RUN: %clang_cc1 %s -include-pch %t -ffreestanding -triple=i686-apple-darwin9 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -emit-pch -o %t -ffreestanding -ffake-address-space-map -triple=i686-apple-darwin9
+// RUN: %clang_cc1 %s -include-pch %t -ffreestanding -ffake-address-space-map -triple=i686-apple-darwin9 -emit-llvm -o - | FileCheck %s
 #ifndef ALREADY_INCLUDED
 #define ALREADY_INCLUDED
 
@@ -155,6 +155,14 @@
   return atomic_compare_exchange_strong(i, &cmp, 1);
 }
 
+#define _AS1 __attribute__((address_space(1)))
+_Bool fi4d(_Atomic(int) *i, int _AS1 *ptr2) {
+  // CHECK-LABEL: @fi4d(
+  // CHECK: [[EXPECTED:%[.0-9A-Z_a-z]+]] = load i32, i32 addrspace(1)* %{{[0-9]+}}
+  // CHECK: cmpxchg i32* %{{[0-9]+}}, i32 [[EXPECTED]], i32 %{{[0-9]+}} acquire acquire
+  return __c11_atomic_compare_exchange_strong(i, ptr2, 1, memory_order_acquire, memory_order_acquire);
+}
+
 float ff1(_Atomic(float) *d) {
   // CHECK-LABEL: @ff1
   // CHECK: load atomic i32, i32* {{.*}} monotonic


Thank you!

Anastasia

-----Original Message-----
From: cfe-commits [mailto:cfe-commits-bounces at lists.llvm.org] On Behalf Of Anastasia Stulova via cfe-commits
Sent: 23 November 2015 10:32
To: cfe-commits at lists.llvm.org; 'Pekka Jääskeläinen'
Subject: RE: [Patch][OpenCL] Custom atomic Builtin check ignores address space of a non-atomic pointer

Ping! Re-attaching the final patch.

-----Original Message-----
From: cfe-commits [mailto:cfe-commits-bounces at lists.llvm.org] On Behalf Of Anastasia Stulova via cfe-commits
Sent: 21 October 2015 11:49
To: 'Pekka Jääskeläinen'; cfe-commits at lists.llvm.org
Subject: RE: [Patch][OpenCL] Custom atomic Builtin check ignores address space of a non-atomic pointer

Hi Pekka,

Are you ok with this change?

Thanks,
Anastasia

-----Original Message-----
From: cfe-commits [mailto:cfe-commits-bounces at lists.llvm.org] On Behalf Of Anastasia Stulova via cfe-commits
Sent: 12 October 2015 16:00
To: 'Pekka Jääskeläinen'; cfe-commits at lists.llvm.org
Subject: RE: [Patch][OpenCL] Custom atomic Builtin check ignores address space of a non-atomic pointer

I have just made one minor update in the CodeGen test (test/CodeGen/atomic-ops.c) that is now checking the IR output rather than only making sure frontend doesn't crash.

The final patch is attached here!

Thanks,
Anastasia
 

-----Original Message-----
From: Pekka Jääskeläinen [mailto:pekka.jaaskelainen at tut.fi]
Sent: 02 October 2015 10:20
To: Anastasia Stulova; cfe-commits at lists.llvm.org
Subject: Re: [Patch][OpenCL] Custom atomic Builtin check ignores address space of a non-atomic pointer

LGTM.

Related to it:
There has been so many getPointerTo() issues with multi-AS in the past that I wonder if it'd be time to drop the default value from it, and go through all the places where it's called with the default AS, thus breaking multi-AS.  Might be a worthwhile job to do at some point.

On 09/30/2015 01:23 PM, Anastasia Stulova via cfe-commits wrote:
> Hi all,
>
> Address spaces are not handled in custom semantic checks of atomic Builtins.
>
> If there are two pointers passed to the Builtin, it doesn't allow the 
> second
>
> (non-atomic) one to be qualified with an address space.
>
> This patch removed this restriction by recording the address space of 
> the
>
> passed pointers while checking its type correctness.
>
> Currently, the following code:
>
> _Atomic int __attribute__((address_space(1))) *A;
>
> int __attribute__((address_space(2))) *B;
>
> ...
>
> ... = __c11_atomic_compare_exchange_strong(A, B, 1, 
> memory_order_seq_cst, memory_order_seq_cst);
>
> fails to compile with an error:
>
> "passing '__attribute__((address_space(2))) int *' to parameter of 
> type 'int *' changes address space of pointer".
>
> Please, review the attached fix for it!
>
> Cheers,
>
> Anastasia
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>

--
Pekka



_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits