Hi Anastasia,
Still LGTM.
Pekka
On 16.12.2015 16:42, Anastasia Stulova wrote:
Hi Pekka,
Re-attaching as a full patch again as something went wrong earlier with the
diff wrapping in the email.
Thanks,
Anastasia
-----Original Message-----
From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
Anastasia Stulova via cfe-commits
Sent: 07 December 2015 17:25
To: 'Pekka Jääskeläinen'; cfe-commits@lists.llvm.org
Subject: RE: [Patch][OpenCL] Custom atomic Builtin check ignores address space
of a non-atomic pointer
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-boun...@lists.llvm.org] On Behalf Of
Anastasia Stulova via cfe-commits
Sent: 23 November 2015 10:32
To: cfe-commits@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-boun...@lists.llvm.org] On Behalf Of
Anastasia Stulova via cfe-commits
Sent: 21 October 2015 11:49
To: 'Pekka Jääskeläinen'; cfe-commits@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-boun...@lists.llvm.org] On Behalf Of
Anastasia Stulova via cfe-commits
Sent: 12 October 2015 16:00
To: 'Pekka Jääskeläinen'; cfe-commits@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.jaaskelai...@tut.fi]
Sent: 02 October 2015 10:20
To: Anastasia Stulova; cfe-commits@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@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
--
Pekka
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
--
Pekka
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits