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

Thu Dec 17 00:35:08 PST 2015

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-bounces at lists.llvm.org] On Behalf Of Anastasia Stulova via cfe-commits
> Sent: 07 December 2015 17:25
> 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
>
> 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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>

-- 
Pekka