[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton added a comment.

In D130123#3691011 , @rnk wrote:

> That sounds reasonable to me, I confirmed that MSVC really only lets you 
> apply these attributes directly to pointer types and to typedefs.
>
> Can you add a test for the other most common type sugar node, the template 
> parameter? It looks like this:
>
>   template 
>   void f(T __ptr32 a) {
>   (*a) += 1;
>   }
>   void g(int *p) {
>   f(p);
>   }
>
> If there isn't already a C++ test for __ptr32 & co, go ahead and make one.

What is your expectation for your template code fragment?  MSVC does not accept 
it.

On the other hand, MSVC does accept this:

  template 
  void f(T  a) {
(*a) += sizeof(a);
  }
  void g(int *p) {
f(p);
  }
  void h(int *__ptr32 p) {
  f(p);
  }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130123/new/

https://reviews.llvm.org/D130123

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton added a comment.

In D130123#3696786 , @rnk wrote:

> In D130123#3696752 , @Ariel-Burton 
> wrote:
>
>> What is your expectation for your template code fragment?  MSVC does not 
>> accept it.
>
> Yes, I checked, MSVC rejects it, so clang should have test expectations to 
> confirm that. It seems interesting or surprising, to me at least, that MSVC 
> really only accepts __ptr32 on pointers and typedefs of them.
>
>> On the other hand, MSVC does accept this:
>>
>>   template 
>>   void f(T  a) {
>> (*a) += sizeof(a);
>>   }
>>   void g(int *p) {
>> f(p);
>>   }
>>   void h(int *__ptr32 p) {
>>   f(p);
>>   }
>
> Right, this makes sense to me. MSVC's diagnostics say something about the 
> __ptr32 qualifier needing to appear after a `*`, so this extension must be 
> implemented at a pretty low-level, with some exception for typedefs, just 
> like what you have.

Thanks.

Just to make sure that we're on the same  page, you'd like to see a test that 
confirms that clang is rejecting the  `template void f(T __ptr32 
a)` example, and possibly one that checks that my example is accepted.  Is that 
correct?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130123/new/

https://reviews.llvm.org/D130123

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton updated this revision to Diff 450153.
Ariel-Burton added a comment.

Added C++ tests for __ptr32.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130123/new/

https://reviews.llvm.org/D130123

Files:
  clang/test/Sema/MicrosoftExtensions.cpp


Index: clang/test/Sema/MicrosoftExtensions.cpp
===
--- clang/test/Sema/MicrosoftExtensions.cpp
+++ clang/test/Sema/MicrosoftExtensions.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -triple i686-windows %s -fsyntax-only -Wmicrosoft -verify 
-fms-extensions
 // RUN: %clang_cc1 -triple x86_64-windows %s -fsyntax-only -Wmicrosoft -verify 
-fms-extensions
-// expected-no-diagnostics
 
 // Check that __ptr32/__ptr64 can be compared.
 int test_ptr_comparison(int *__ptr32 __uptr p32u, int *__ptr32 __sptr p32s,
@@ -9,3 +8,23 @@
  (p32u == p64) +
  (p32s == p64);
 }
+
+template
+void bad(T __ptr32 a) { // expected-error {{'__ptr32' attribute only applies 
to pointer arguments}}`
+  (*a) += 1;
+}
+
+template
+void f(T a) {
+  (*a) += sizeof(a);
+  static_assert(sizeof(a) == size_expected, "instantiated template argument 
has unexpected size");
+}
+void g(int *p) {
+  // instantiate for default sized pointer
+  f(p);
+}
+
+void h(int *__ptr32 p) {
+  // instantiate for 32-bit pointer
+  f<4>(p);
+}


Index: clang/test/Sema/MicrosoftExtensions.cpp
===
--- clang/test/Sema/MicrosoftExtensions.cpp
+++ clang/test/Sema/MicrosoftExtensions.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -triple i686-windows %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 // RUN: %clang_cc1 -triple x86_64-windows %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
 
 // Check that __ptr32/__ptr64 can be compared.
 int test_ptr_comparison(int *__ptr32 __uptr p32u, int *__ptr32 __sptr p32s,
@@ -9,3 +8,23 @@
  (p32u == p64) +
  (p32s == p64);
 }
+
+template
+void bad(T __ptr32 a) { // expected-error {{'__ptr32' attribute only applies to pointer arguments}}`
+  (*a) += 1;
+}
+
+template
+void f(T a) {
+  (*a) += sizeof(a);
+  static_assert(sizeof(a) == size_expected, "instantiated template argument has unexpected size");
+}
+void g(int *p) {
+  // instantiate for default sized pointer
+  f(p);
+}
+
+void h(int *__ptr32 p) {
+  // instantiate for 32-bit pointer
+  f<4>(p);
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton updated this revision to Diff 450157.
Ariel-Burton added a comment.

Recover original changes, and commit with new C++ __ptr32 test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130123/new/

https://reviews.llvm.org/D130123

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/address-space-ptr32.c
  clang/test/Sema/MicrosoftExtensions.c
  clang/test/Sema/MicrosoftExtensions.cpp

Index: clang/test/Sema/MicrosoftExtensions.cpp
===
--- clang/test/Sema/MicrosoftExtensions.cpp
+++ clang/test/Sema/MicrosoftExtensions.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -triple i686-windows %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 // RUN: %clang_cc1 -triple x86_64-windows %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
 
 // Check that __ptr32/__ptr64 can be compared.
 int test_ptr_comparison(int *__ptr32 __uptr p32u, int *__ptr32 __sptr p32s,
@@ -9,3 +8,23 @@
  (p32u == p64) +
  (p32s == p64);
 }
+
+template
+void bad(T __ptr32 a) { // expected-error {{'__ptr32' attribute only applies to pointer arguments}}`
+  (*a) += 1;
+}
+
+template
+void f(T a) {
+  (*a) += sizeof(a);
+  static_assert(sizeof(a) == size_expected, "instantiated template argument has unexpected size");
+}
+void g(int *p) {
+  // instantiate for default sized pointer
+  f(p);
+}
+
+void h(int *__ptr32 p) {
+  // instantiate for 32-bit pointer
+  f<4>(p);
+}
Index: clang/test/Sema/MicrosoftExtensions.c
===
--- clang/test/Sema/MicrosoftExtensions.c
+++ clang/test/Sema/MicrosoftExtensions.c
@@ -173,8 +173,28 @@
 
 int *(__ptr32 __sptr wrong9); // expected-error {{'__sptr' attribute only applies to pointer arguments}} // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
 
+int *(__ptr32 wrong10); // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
+
+int *(__ptr64 wrong11); // expected-error {{'__ptr64' attribute only applies to pointer arguments}}
+
+int *(__ptr32 __ptr64 wrong12); // expected-error {{'__ptr32' attribute only applies to pointer arguments}} // expected-error {{'__ptr64' attribute only applies to pointer arguments}}
+
 typedef int *T;
-T __ptr32 wrong10; // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
+T __ptr32 ok1;
+T __ptr64 ok2;
+T __ptr32 __ptr64 wrong13; // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr32 T1;
+T1 ok3;
+T1 __ptr32 wrong14;  // expected-warning {{attribute '__ptr32' is already applied}}
+T1 __ptr64 wrong15;  // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr64 T2;
+T2 ok4;
+T2 __ptr64 wrong16;  // expected-warning {{attribute '__ptr64' is already applied}}
+T2 __ptr32 wrong17;  // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr32 __ptr64 wrong18; // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
 
 typedef char *my_va_list;
 void __va_start(my_va_list *ap, ...); // expected-note {{passing argument to parameter 'ap' here}}
Index: clang/test/CodeGen/address-space-ptr32.c
===
--- clang/test/CodeGen/address-space-ptr32.c
+++ clang/test/CodeGen/address-space-ptr32.c
@@ -1,10 +1,40 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-windows-msvc -fms-extensions -emit-llvm < %s | FileCheck %s
 
+_Static_assert(sizeof(void *) == 8, "sizeof(void *) has unexpected value.  Expected 8.");
+
 int foo(void) {
+  // CHECK: define dso_local i32 @foo
+  // CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
+  // CHECK: ret i32 4
   int (*__ptr32 a)(int);
   return sizeof(a);
 }
 
-// CHECK: define dso_local i32 @foo
-// CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
-// CHECK: ret i32 4
+int bar(void) {
+  // CHECK: define dso_local i32 @bar
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  int *__ptr32 p;
+  return sizeof(p);
+}
+
+
+int baz(void) {
+  // CHECK: define dso_local i32 @baz
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *__ptr32 IP32_PTR;
+
+  IP32_PTR p;
+  return sizeof(p);
+}
+
+int fugu(void) {
+  // CHECK: define dso_local i32 @fugu
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *int_star;
+
+  int_star __ptr32 p;
+  return sizeof(p);
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7158,17 +7158,22 @@
   }
 
   std::bitset Attrs;
-  attr::Kind NewAttrKind = A->getKind();
   QualType Desugared = Type;
-  const AttributedType *AT = dyn_cast(Type);
-  while (AT) {
+  for (;;) {
+if (const TypedefType *TT = dy

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton added a comment.

In D130123#3700174 , @rnk wrote:

> In D130123#3698399 , @Ariel-Burton 
> wrote:
>
>> Just to make sure that we're on the same  page, you'd like to see a test 
>> that confirms that clang is rejecting the  `template void f(T 
>> __ptr32 a)` example, and possibly one that checks that my example is 
>> accepted.  Is that correct?
>
> Yep, exactly that. Sorry for the delay, the code looks good.

Please see updated test `clang/test/Sema/MicrosoftExtensions.cpp`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130123/new/

https://reviews.llvm.org/D130123

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton updated this revision to Diff 450982.
Ariel-Burton added a comment.

- Add case to deal with ElaboratedTypes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130123/new/

https://reviews.llvm.org/D130123

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/address-space-ptr32.c
  clang/test/Sema/MicrosoftExtensions.c
  clang/test/Sema/MicrosoftExtensions.cpp

Index: clang/test/Sema/MicrosoftExtensions.cpp
===
--- clang/test/Sema/MicrosoftExtensions.cpp
+++ clang/test/Sema/MicrosoftExtensions.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -triple i686-windows %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 // RUN: %clang_cc1 -triple x86_64-windows %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
 
 // Check that __ptr32/__ptr64 can be compared.
 int test_ptr_comparison(int *__ptr32 __uptr p32u, int *__ptr32 __sptr p32s,
@@ -9,3 +8,23 @@
  (p32u == p64) +
  (p32s == p64);
 }
+
+template
+void bad(T __ptr32 a) { // expected-error {{'__ptr32' attribute only applies to pointer arguments}}`
+  (*a) += 1;
+}
+
+template
+void f(T a) {
+  (*a) += sizeof(a);
+  static_assert(sizeof(a) == size_expected, "instantiated template argument has unexpected size");
+}
+void g(int *p) {
+  // instantiate for default sized pointer
+  f(p);
+}
+
+void h(int *__ptr32 p) {
+  // instantiate for 32-bit pointer
+  f<4>(p);
+}
Index: clang/test/Sema/MicrosoftExtensions.c
===
--- clang/test/Sema/MicrosoftExtensions.c
+++ clang/test/Sema/MicrosoftExtensions.c
@@ -173,8 +173,28 @@
 
 int *(__ptr32 __sptr wrong9); // expected-error {{'__sptr' attribute only applies to pointer arguments}} // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
 
+int *(__ptr32 wrong10); // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
+
+int *(__ptr64 wrong11); // expected-error {{'__ptr64' attribute only applies to pointer arguments}}
+
+int *(__ptr32 __ptr64 wrong12); // expected-error {{'__ptr32' attribute only applies to pointer arguments}} // expected-error {{'__ptr64' attribute only applies to pointer arguments}}
+
 typedef int *T;
-T __ptr32 wrong10; // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
+T __ptr32 ok1;
+T __ptr64 ok2;
+T __ptr32 __ptr64 wrong13; // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr32 T1;
+T1 ok3;
+T1 __ptr32 wrong14;  // expected-warning {{attribute '__ptr32' is already applied}}
+T1 __ptr64 wrong15;  // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr64 T2;
+T2 ok4;
+T2 __ptr64 wrong16;  // expected-warning {{attribute '__ptr64' is already applied}}
+T2 __ptr32 wrong17;  // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr32 __ptr64 wrong18; // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
 
 typedef char *my_va_list;
 void __va_start(my_va_list *ap, ...); // expected-note {{passing argument to parameter 'ap' here}}
Index: clang/test/CodeGen/address-space-ptr32.c
===
--- clang/test/CodeGen/address-space-ptr32.c
+++ clang/test/CodeGen/address-space-ptr32.c
@@ -1,10 +1,40 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-windows-msvc -fms-extensions -emit-llvm < %s | FileCheck %s
 
+_Static_assert(sizeof(void *) == 8, "sizeof(void *) has unexpected value.  Expected 8.");
+
 int foo(void) {
+  // CHECK: define dso_local i32 @foo
+  // CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
+  // CHECK: ret i32 4
   int (*__ptr32 a)(int);
   return sizeof(a);
 }
 
-// CHECK: define dso_local i32 @foo
-// CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
-// CHECK: ret i32 4
+int bar(void) {
+  // CHECK: define dso_local i32 @bar
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  int *__ptr32 p;
+  return sizeof(p);
+}
+
+
+int baz(void) {
+  // CHECK: define dso_local i32 @baz
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *__ptr32 IP32_PTR;
+
+  IP32_PTR p;
+  return sizeof(p);
+}
+
+int fugu(void) {
+  // CHECK: define dso_local i32 @fugu
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *int_star;
+
+  int_star __ptr32 p;
+  return sizeof(p);
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7158,17 +7158,25 @@
   }
 
   std::bitset Attrs;
-  attr::Kind NewAttrKind = A->getKind();
   QualType Desugared = Type;
-  const AttributedType *AT = dyn_cast(Type);
-  while (AT) {
+  for (;;) {
+if (const TypedefType *TT = dyn_cast(Desugared)) {
+ 

[PATCH] D130123: [Phabricator] extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton created this revision.
Ariel-Burton added reviewers: akhuang, rnk, rsmith.
Herald added a project: All.
Ariel-Burton requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Earlier, if the QualType was sugared, then we would error out
as it was not a pointer type, for example,

typedef int *int_star;

int_star __ptr32  p;

Now, if ptr32 is given we apply it if the raw Canonical Type
(i.e., the desugared type) is a PointerType, instead of only
checking whether the sugared type is a pointer type.

As before, we still disallow ptr32 usage if the pointer is used
as a pointer to a member.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130123

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/address-space-ptr32.c
  clang/test/Sema/MicrosoftExtensions.c

Index: clang/test/Sema/MicrosoftExtensions.c
===
--- clang/test/Sema/MicrosoftExtensions.c
+++ clang/test/Sema/MicrosoftExtensions.c
@@ -173,8 +173,28 @@
 
 int *(__ptr32 __sptr wrong9); // expected-error {{'__sptr' attribute only applies to pointer arguments}} // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
 
+int *(__ptr32 wrong10); // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
+
+int *(__ptr64 wrong11); // expected-error {{'__ptr64' attribute only applies to pointer arguments}}
+
+int *(__ptr32 __ptr64 wrong12); // expected-error {{'__ptr32' attribute only applies to pointer arguments}} // expected-error {{'__ptr64' attribute only applies to pointer arguments}}
+
 typedef int *T;
-T __ptr32 wrong10; // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
+T __ptr32 ok1;
+T __ptr64 ok2;
+T __ptr32 __ptr64 wrong13; // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr32 T1;
+T1 ok3;
+T1 __ptr32 wrong14;  // expected-warning {{attribute '__ptr32' is already applied}}
+T1 __ptr64 wrong15;  // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr64 T2;
+T2 ok4;
+T2 __ptr64 wrong16;  // expected-warning {{attribute '__ptr64' is already applied}}
+T2 __ptr32 wrong17;  // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr32 __ptr64 wrong18; // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
 
 typedef char *my_va_list;
 void __va_start(my_va_list *ap, ...); // expected-note {{passing argument to parameter 'ap' here}}
Index: clang/test/CodeGen/address-space-ptr32.c
===
--- clang/test/CodeGen/address-space-ptr32.c
+++ clang/test/CodeGen/address-space-ptr32.c
@@ -1,10 +1,40 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-windows-msvc -fms-extensions -emit-llvm < %s | FileCheck %s
 
+_Static_assert(sizeof(void *) == 8, "sizeof(void *) has unexpected value.  Expected 8.");
+
 int foo(void) {
+  // CHECK: define dso_local i32 @foo
+  // CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
+  // CHECK: ret i32 4
   int (*__ptr32 a)(int);
   return sizeof(a);
 }
 
-// CHECK: define dso_local i32 @foo
-// CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
-// CHECK: ret i32 4
+int bar(void) {
+  // CHECK: define dso_local i32 @bar
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  int *__ptr32 p;
+  return sizeof(p);
+}
+
+
+int baz(void) {
+  // CHECK: define dso_local i32 @baz
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *__ptr32 IP32_PTR;
+
+  IP32_PTR p;
+  return sizeof(p);
+}
+
+int fugu(void) {
+  // CHECK: define dso_local i32 @fugu
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *int_star;
+
+  int_star __ptr32 p;
+  return sizeof(p);
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7111,17 +7111,22 @@
   }
 
   std::bitset Attrs;
-  attr::Kind NewAttrKind = A->getKind();
   QualType Desugared = Type;
-  const AttributedType *AT = dyn_cast(Type);
-  while (AT) {
+  for (;;) {
+if (const TypedefType *TT = dyn_cast(Desugared)) {
+  Desugared = TT->desugar();
+  continue;
+}
+const AttributedType *AT = dyn_cast(Desugared);
+if (!AT)
+  break;
 Attrs[AT->getAttrKind()] = true;
 Desugared = AT->getModifiedType();
-AT = dyn_cast(Desugared);
   }
 
   // You cannot specify duplicate type attributes, so if the attribute has
   // already been applied, flag it.
+  attr::Kind NewAttrKind = A->getKind();
   if (Attrs[NewAttrKind]) {
 S.Diag(PAttr.getLoc(), diag::warn_duplicate_attribute_exact) << PAttr;
 return true;
@@ -7142,14 +7147,11 @@
 return true;
   }
 
-  // Pointer type qualifiers can only operate on pointer types, but 

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:7116
+  for (;;) {
+if (const TypedefType *TT = dyn_cast(Desugared)) {
+  Desugared = TT->desugar();

rnk wrote:
> This seems like a good place to use getSingleStepDesugaredType to look 
> through all type sugar (parens, typedefs, template substitutions, etc).
> This seems like a good place to use getSingleStepDesugaredType to look 
> through all type sugar (parens, typedefs, template substitutions, etc).

I'm not sure what you mean.  Could you expand a little, please?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130123/new/

https://reviews.llvm.org/D130123

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:7116
+  for (;;) {
+if (const TypedefType *TT = dyn_cast(Desugared)) {
+  Desugared = TT->desugar();

rnk wrote:
> Ariel-Burton wrote:
> > rnk wrote:
> > > This seems like a good place to use getSingleStepDesugaredType to look 
> > > through all type sugar (parens, typedefs, template substitutions, etc).
> > > This seems like a good place to use getSingleStepDesugaredType to look 
> > > through all type sugar (parens, typedefs, template substitutions, etc).
> > 
> > I'm not sure what you mean.  Could you expand a little, please?
> Clang's AST has lots of "type sugar nodes". These are types which usually 
> don't have any semantic meaning, they just carry source location information, 
> like whether there was a typedef or extra parens in the type. AttributedType 
> is also a type sugar node, so we cannot do a full desugaring here, we have to 
> step through each node one at a time to accumulate the attributes.
> 
> Your code looks through one kind of type sugar, but this loop should probably 
> be generalized to handle all kinds of type sugar. I think 
> getSingleStepDesugaredType will do that.
Thanks for the clarification.  Do you mean something like:

 
```
 for (;;) {
const AttributedType *AT = dyn_cast(Desugared);
if (AT) {
  Attrs[AT->getAttrKind()] = true;
  Desugared = AT->getModifiedType();
} else {
  QualType QT = Desugared.getSingleStepDesugaredType(S.Context);
  if (Desugared != QT) {
Desugared = QT;
  } else {
break;
  }
}
  }
```

I don't think that we can use getSingleStepDesugaredType indiscriminately.  Not 
all sugar should be peeled away, for example, in the case of of parentheses:


```
int *(__ptr32 wrong);
```

is accepted when it shouldn't.

To check for the cases where we don't want to desugar has the same sort of 
complexity as checking for the cases where we do.  I suggest going with the 
original proposal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130123/new/

https://reviews.llvm.org/D130123

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142048: [Phabricator] Fix __ptr32 arguments passed to builtins

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton created this revision.
Herald added subscribers: arichardson, Anastasia.
Herald added a project: All.
Ariel-Burton requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently when clang deals with a call to a builtin function that
is supplied with an argument that has an explicit address space
it rewrites the signature of the callee to make the types of
the formal parameters match those of the actual arguments.
This functionality was added to support OpenCL, and was
introduced with commit b919c7d.

However, this does not work properly for "size" related address
spaces such as those used for __ptr32. This affects platforms
like Microsoft and z/OS.

This change preserves the OpenCL functionality, but will use
the formal parameter types when an address space is size-related.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142048

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/address-space-ptr32.c


Index: clang/test/CodeGen/address-space-ptr32.c
===
--- clang/test/CodeGen/address-space-ptr32.c
+++ clang/test/CodeGen/address-space-ptr32.c
@@ -38,3 +38,31 @@
   int_star __ptr32 p;
   return sizeof(p);
 }
+
+typedef __SIZE_TYPE__ size_t;
+size_t strlen(const char *);
+
+size_t test_calling_strlen_with_32_bit_pointer ( char *__ptr32 s ) {
+  // CHECK-LABEL: define dso_local i64 
@test_calling_strlen_with_32_bit_pointer(ptr addrspace(270) noundef %s)
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:   %s.addr = alloca ptr addrspace(270), align 4
+  // CHECK-NEXT:   store ptr addrspace(270) %s, ptr %s.addr, align 4
+  // CHECK-NEXT:   %0 = load ptr addrspace(270), ptr %s.addr, align 4
+  // CHECK-NEXT:   %1 = addrspacecast ptr addrspace(270) %0 to ptr
+  // CHECK-NEXT:   %call = call i64 @strlen(ptr  noundef %1)
+  // CHECK-NEXT:   ret i64 %call
+   return strlen ( s );
+}
+
+// CHECK-LABEL: declare dso_local i64 @strlen(ptr noundef)
+
+size_t test_calling_strlen_with_64_bit_pointer ( char *s ) {
+  // CHECK-LABEL: define dso_local i64 
@test_calling_strlen_with_64_bit_pointer(ptr noundef %s)
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:   %s.addr = alloca ptr, align 8
+  // CHECK-NEXT:   store ptr %s, ptr %s.addr, align 8
+  // CHECK-NEXT:   %0 = load ptr, ptr %s.addr, align 8
+  // CHECK-NEXT:   %call = call i64 @strlen(ptr noundef %0)
+  // CHECK-NEXT:   ret i64 %call
+  return strlen ( s );
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -6653,7 +6653,8 @@
 if (!ParamType->isPointerType() ||
 ParamType.hasAddressSpace() ||
 !ArgType->isPointerType() ||
-!ArgType->getPointeeType().hasAddressSpace()) {
+!ArgType->getPointeeType().hasAddressSpace() ||
+isPtrSizeAddressSpace(ArgType->getPointeeType().getAddressSpace())) {
   OverloadParams.push_back(ParamType);
   continue;
 }


Index: clang/test/CodeGen/address-space-ptr32.c
===
--- clang/test/CodeGen/address-space-ptr32.c
+++ clang/test/CodeGen/address-space-ptr32.c
@@ -38,3 +38,31 @@
   int_star __ptr32 p;
   return sizeof(p);
 }
+
+typedef __SIZE_TYPE__ size_t;
+size_t strlen(const char *);
+
+size_t test_calling_strlen_with_32_bit_pointer ( char *__ptr32 s ) {
+  // CHECK-LABEL: define dso_local i64 @test_calling_strlen_with_32_bit_pointer(ptr addrspace(270) noundef %s)
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:   %s.addr = alloca ptr addrspace(270), align 4
+  // CHECK-NEXT:   store ptr addrspace(270) %s, ptr %s.addr, align 4
+  // CHECK-NEXT:   %0 = load ptr addrspace(270), ptr %s.addr, align 4
+  // CHECK-NEXT:   %1 = addrspacecast ptr addrspace(270) %0 to ptr
+  // CHECK-NEXT:   %call = call i64 @strlen(ptr  noundef %1)
+  // CHECK-NEXT:   ret i64 %call
+   return strlen ( s );
+}
+
+// CHECK-LABEL: declare dso_local i64 @strlen(ptr noundef)
+
+size_t test_calling_strlen_with_64_bit_pointer ( char *s ) {
+  // CHECK-LABEL: define dso_local i64 @test_calling_strlen_with_64_bit_pointer(ptr noundef %s)
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:   %s.addr = alloca ptr, align 8
+  // CHECK-NEXT:   store ptr %s, ptr %s.addr, align 8
+  // CHECK-NEXT:   %0 = load ptr, ptr %s.addr, align 8
+  // CHECK-NEXT:   %call = call i64 @strlen(ptr noundef %0)
+  // CHECK-NEXT:   ret i64 %call
+  return strlen ( s );
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -6653,7 +6653,8 @@
 if (!ParamType->isPointerType() ||
 ParamType.hasAddressSpace() ||
 !ArgType->isPointerType() ||
-!ArgType->getPointeeType().hasAddressSpace()) {
+!ArgType->getPointeeType().hasAddressSpace() ||
+isPtrSizeAddressSpace(ArgType->getPointee

[PATCH] D142048: [Phabricator] Fix __ptr32 arguments passed to builtins

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton updated this revision to Diff 490569.
Ariel-Burton added a comment.

- Adjust formatting to conform to git-clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142048/new/

https://reviews.llvm.org/D142048

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/address-space-ptr32.c


Index: clang/test/CodeGen/address-space-ptr32.c
===
--- clang/test/CodeGen/address-space-ptr32.c
+++ clang/test/CodeGen/address-space-ptr32.c
@@ -38,3 +38,31 @@
   int_star __ptr32 p;
   return sizeof(p);
 }
+
+typedef __SIZE_TYPE__ size_t;
+size_t strlen(const char *);
+
+size_t test_calling_strlen_with_32_bit_pointer ( char *__ptr32 s ) {
+  // CHECK-LABEL: define dso_local i64 
@test_calling_strlen_with_32_bit_pointer(ptr addrspace(270) noundef %s)
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:   %s.addr = alloca ptr addrspace(270), align 4
+  // CHECK-NEXT:   store ptr addrspace(270) %s, ptr %s.addr, align 4
+  // CHECK-NEXT:   %0 = load ptr addrspace(270), ptr %s.addr, align 4
+  // CHECK-NEXT:   %1 = addrspacecast ptr addrspace(270) %0 to ptr
+  // CHECK-NEXT:   %call = call i64 @strlen(ptr  noundef %1)
+  // CHECK-NEXT:   ret i64 %call
+   return strlen ( s );
+}
+
+// CHECK-LABEL: declare dso_local i64 @strlen(ptr noundef)
+
+size_t test_calling_strlen_with_64_bit_pointer ( char *s ) {
+  // CHECK-LABEL: define dso_local i64 
@test_calling_strlen_with_64_bit_pointer(ptr noundef %s)
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:   %s.addr = alloca ptr, align 8
+  // CHECK-NEXT:   store ptr %s, ptr %s.addr, align 8
+  // CHECK-NEXT:   %0 = load ptr, ptr %s.addr, align 8
+  // CHECK-NEXT:   %call = call i64 @strlen(ptr noundef %0)
+  // CHECK-NEXT:   ret i64 %call
+  return strlen ( s );
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -6650,10 +6650,10 @@
   return nullptr;
 Expr *Arg = ArgRes.get();
 QualType ArgType = Arg->getType();
-if (!ParamType->isPointerType() ||
-ParamType.hasAddressSpace() ||
+if (!ParamType->isPointerType() || ParamType.hasAddressSpace() ||
 !ArgType->isPointerType() ||
-!ArgType->getPointeeType().hasAddressSpace()) {
+!ArgType->getPointeeType().hasAddressSpace() ||
+isPtrSizeAddressSpace(ArgType->getPointeeType().getAddressSpace())) {
   OverloadParams.push_back(ParamType);
   continue;
 }


Index: clang/test/CodeGen/address-space-ptr32.c
===
--- clang/test/CodeGen/address-space-ptr32.c
+++ clang/test/CodeGen/address-space-ptr32.c
@@ -38,3 +38,31 @@
   int_star __ptr32 p;
   return sizeof(p);
 }
+
+typedef __SIZE_TYPE__ size_t;
+size_t strlen(const char *);
+
+size_t test_calling_strlen_with_32_bit_pointer ( char *__ptr32 s ) {
+  // CHECK-LABEL: define dso_local i64 @test_calling_strlen_with_32_bit_pointer(ptr addrspace(270) noundef %s)
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:   %s.addr = alloca ptr addrspace(270), align 4
+  // CHECK-NEXT:   store ptr addrspace(270) %s, ptr %s.addr, align 4
+  // CHECK-NEXT:   %0 = load ptr addrspace(270), ptr %s.addr, align 4
+  // CHECK-NEXT:   %1 = addrspacecast ptr addrspace(270) %0 to ptr
+  // CHECK-NEXT:   %call = call i64 @strlen(ptr  noundef %1)
+  // CHECK-NEXT:   ret i64 %call
+   return strlen ( s );
+}
+
+// CHECK-LABEL: declare dso_local i64 @strlen(ptr noundef)
+
+size_t test_calling_strlen_with_64_bit_pointer ( char *s ) {
+  // CHECK-LABEL: define dso_local i64 @test_calling_strlen_with_64_bit_pointer(ptr noundef %s)
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:   %s.addr = alloca ptr, align 8
+  // CHECK-NEXT:   store ptr %s, ptr %s.addr, align 8
+  // CHECK-NEXT:   %0 = load ptr, ptr %s.addr, align 8
+  // CHECK-NEXT:   %call = call i64 @strlen(ptr noundef %0)
+  // CHECK-NEXT:   ret i64 %call
+  return strlen ( s );
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -6650,10 +6650,10 @@
   return nullptr;
 Expr *Arg = ArgRes.get();
 QualType ArgType = Arg->getType();
-if (!ParamType->isPointerType() ||
-ParamType.hasAddressSpace() ||
+if (!ParamType->isPointerType() || ParamType.hasAddressSpace() ||
 !ArgType->isPointerType() ||
-!ArgType->getPointeeType().hasAddressSpace()) {
+!ArgType->getPointeeType().hasAddressSpace() ||
+isPtrSizeAddressSpace(ArgType->getPointeeType().getAddressSpace())) {
   OverloadParams.push_back(ParamType);
   continue;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits