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

[Kai Nacke via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd1d93da701b1: [Clang] Fix __ptr32 arguments passed to 
builtins (authored by Ariel-Burton, committed by Kai).

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
@@ -6655,10 +6655,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
@@ -6655,10 +6655,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-c

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

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

It sounds to me that the logic from b919c7d should have been restricted to 
actual LLVM intrinsics that are overloaded on the pointer args ( e.g. 
llvm.memcpy which was the test in that commit). Builtins that are just library 
calls with known semantics probably need the cast even for opencl?




Comment at: clang/test/CodeGen/address-space-ptr32.c:54
+  // CHECK-NEXT:   ret i64 %call
+   return strlen ( s );
+}

Maybe use `__builtin_strlen` here instead to avoid the declaration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142048

___
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 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

[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