[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
This revision was automatically updated to reflect the committed changes.
Closed by commit rL289252: Add support for non-zero null pointer for C and
OpenCL (authored by yaxunl).
Changed prior to commit:
https://reviews.llvm.org/D26196?vs=80902=80919#toc
Repository:
rL LLVM
https://reviews.llvm.org/D26196
Files:
cfe/trunk/include/clang/AST/APValue.h
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/AST/APValue.cpp
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
cfe/trunk/lib/CodeGen/CodeGenTypes.h
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/CodeGen/TargetInfo.h
cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
Index: cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -0,0 +1,534 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -emit-llvm -o - | FileCheck --check-prefix=NOOPT %s
+
+typedef struct {
+ private char *p1;
+ local char *p2;
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy1;
+
+typedef struct {
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy2;
+
+// LLVM requests global variable with common linkage to be initialized with zeroinitializer, therefore use -fno-common
+// to suppress common linkage for tentative definition.
+
+// Test 0 as initializer.
+
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p = 0;
+
+// CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p = 0;
+
+// CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p = 0;
+
+// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p = 0;
+
+// CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p = 0;
+
+// Test NULL as initializer.
+
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p_NULL = NULL;
+
+// CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p_NULL = NULL;
+
+// CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p_NULL = NULL;
+
+// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p_NULL = NULL;
+
+// CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p_NULL = NULL;
+
+// Test constant folding of null pointer.
+// A null pointer should be folded to a null pointer in the target address space.
+
+// CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 addrspace(4)* null, align 4
+generic int *fold_generic = (global int*)(generic float*)(private char*)0;
+
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* addrspacecast (i16 addrspace(4)* null to i16*), align 4
+private short *fold_priv = (private short*)(generic int*)(global void*)0;
+
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* inttoptr (i32 9 to i8*), align 4
+private char *fold_priv_arith = (private char*)0 + 10;
+
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
+int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
+
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
+int fold_int2 = (int) ((private void*)0 + 13);
+
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
+int fold_int3 = (int) ((private int*)0);
+
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
+int fold_int4 = (int) &((private int*)0)[2];
+
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
+int fold_int5 = (int) &((private StructTy1*)0)->p2;
+
+// Test static variable initialization.
+
+// NOOPT: @test_static_var.sp1 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp2 = internal
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Okay. With that resolved, this LGTM. https://reviews.llvm.org/D26196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl updated this revision to Diff 80902.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Revised by John's comments.
Dropped constant folding of ptrtoint of non-zero null pointer.
https://reviews.llvm.org/D26196
Files:
include/clang/AST/APValue.h
include/clang/AST/ASTContext.h
include/clang/Basic/TargetInfo.h
lib/AST/APValue.cpp
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/Basic/Targets.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/CodeGenTypes.h
lib/CodeGen/TargetInfo.cpp
lib/CodeGen/TargetInfo.h
test/CodeGenOpenCL/amdgpu-nullptr.cl
Index: test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -0,0 +1,534 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -emit-llvm -o - | FileCheck --check-prefix=NOOPT %s
+
+typedef struct {
+ private char *p1;
+ local char *p2;
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy1;
+
+typedef struct {
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy2;
+
+// LLVM requests global variable with common linkage to be initialized with zeroinitializer, therefore use -fno-common
+// to suppress common linkage for tentative definition.
+
+// Test 0 as initializer.
+
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p = 0;
+
+// CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p = 0;
+
+// CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p = 0;
+
+// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p = 0;
+
+// CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p = 0;
+
+// Test NULL as initializer.
+
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p_NULL = NULL;
+
+// CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p_NULL = NULL;
+
+// CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p_NULL = NULL;
+
+// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p_NULL = NULL;
+
+// CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p_NULL = NULL;
+
+// Test constant folding of null pointer.
+// A null pointer should be folded to a null pointer in the target address space.
+
+// CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 addrspace(4)* null, align 4
+generic int *fold_generic = (global int*)(generic float*)(private char*)0;
+
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* addrspacecast (i16 addrspace(4)* null to i16*), align 4
+private short *fold_priv = (private short*)(generic int*)(global void*)0;
+
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* inttoptr (i32 9 to i8*), align 4
+private char *fold_priv_arith = (private char*)0 + 10;
+
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
+int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
+
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
+int fold_int2 = (int) ((private void*)0 + 13);
+
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
+int fold_int3 = (int) ((private int*)0);
+
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
+int fold_int4 = (int) &((private int*)0)[2];
+
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
+int fold_int5 = (int) &((private StructTy1*)0)->p2;
+
+// Test static variable initialization.
+
+// NOOPT: @test_static_var.sp1 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp2 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp3 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp4 = internal addrspace(1) global i8* null, align 4
+// NOOPT: @test_static_var.sp5 =
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl marked an inline comment as done.
yaxunl added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1532
+ return llvm::ConstantInt::get(ConvertType(DestTy),
+ CGF.getContext().getTargetNullPtrValue(E->getType()));
assert(!DestTy->isBooleanType() && "bool should use PointerToBool");
rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > yaxunl wrote:
> > > > rjmccall wrote:
> > > > > yaxunl wrote:
> > > > > > rjmccall wrote:
> > > > > > > Why is this necessary? ptrtoint on the recursively-emitted null
> > > > > > > pointer should do this automatically.
> > > > > > Since the target knows the value in the null pointers, it can fold
> > > > > > a null pointer to integer literal directly.
> > > > > >
> > > > > > The above code does that, e.g.
> > > > > >
> > > > > > ```
> > > > > > void test_cast_null_pointer_to_sizet_calee(size_t arg_private,
> > > > > >size_t arg_local,
> > > > > >size_t arg_global,
> > > > > >size_t arg_constant,
> > > > > >size_t arg_generic);
> > > > > >
> > > > > > // CHECK-LABEL: test_cast_null_pointer_to_sizet
> > > > > > // CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 -1,
> > > > > > i64 -1, i64 0, i64 0, i64 0)
> > > > > > void test_cast_null_pointer_to_sizet(void) {
> > > > > > test_cast_null_pointer_to_sizet_calee((size_t)((private char*)0),
> > > > > > (size_t)((local char*)0),
> > > > > > (size_t)((global char*)0),
> > > > > > (size_t)((constant char*)0),
> > > > > > (size_t)((generic char*)0));
> > > > > > }
> > > > > >
> > > > > > ```
> > > > > >
> > > > > > Without the above code, we only get ptrtoint instructions.
> > > > > Oh, does the constant folder not know how to fold
> > > > > ptrtoint(inttoptr(X)) -> X? I guess that's probably LLVM's nominal
> > > > > target-independence rearing its head.
> > > > >
> > > > > Is getting a slightly LLVM constant actually important here? I would
> > > > > prefer to avoid this complexity (and unnecessary recursive walk) if
> > > > > possible.
> > > > Since it's target dependent, it won't be constant folded by the target
> > > > agnostic passes until very late in the backend, which may lose
> > > > optimization opportunities. I think it's better folded by FE like other
> > > > constants.
> > > Are you sure? All of my attempts to produce these constants in LLVM seem
> > > to get instantly folded, even without a target set in the IR:
> > > i32 ptrtoint (i8* inttoptr (i32 -1 to i8*) to i32)
> > > i64 ptrtoint (i8* inttoptr (i64 -1 to i8*) to i64)
> > > This folding literally occurs within ConstantExpr::getPtrToInt, without
> > > any passes running, and it seems to happen regardless of the pointer
> > > having an addrspace.
> > >
> > > I suspect that the problem is that you additionally have an addrspacecast
> > > constant in place, which almost certainly does block the constant folder.
> > > The right fix for that is to ensure that your target's implementation of
> > > performAddrSpaceCast makes an effort to peephole casts of Constants.
> > In amdgpu target, null pointer in addr space 0 is represented as
> > `addrspacecast int addrspace(4)* null to int*` instead of `inttoptr i32 -1
> > to i8*`. As addrspacecast is opaque to target-independent LLVM passes, they
> > don't know how to fold it. Only the backend knows how to fold it.
> >
> > This could be a general situation, i.e., the non-zero null pointer is
> > represented in some target specific form which LLVM does not know how to
> > fold.
> LLVM has deeply-baked-in assumptions that null is a zero bit pattern. It is
> really, really not a good idea to assume that LLVM will never make any
> assumptions about the behavior of addrspacecasts in its target-independent
> code.
>
> Also, it is almost certainly possible to validly produce that constant with
> the expectation that it will have value 0, and thus you special-case lowering
> in the backend would be a miscompile.
>
> I understand that you probably got into this situation by trying to give null
> a non-zero representation in the backend and then slowly realized that you
> needed frontend changes to avoid miscompiles. That is totally
> understandable. Please understand that what are you actually doing now with
> this patch is fundamentally changing where you lower null pointers, so that
> it's now only the frontend which knows about the representation of null
> pointers, and the backend just treats ConstantPointerNull as a special way of
> spelling the valid, semantically non-null zero representation of a pointer.
Thanks John. I think your concern is valid. Actually I
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1532
+ return llvm::ConstantInt::get(ConvertType(DestTy),
+ CGF.getContext().getTargetNullPtrValue(E->getType()));
assert(!DestTy->isBooleanType() && "bool should use PointerToBool");
yaxunl wrote:
> rjmccall wrote:
> > yaxunl wrote:
> > > rjmccall wrote:
> > > > yaxunl wrote:
> > > > > rjmccall wrote:
> > > > > > Why is this necessary? ptrtoint on the recursively-emitted null
> > > > > > pointer should do this automatically.
> > > > > Since the target knows the value in the null pointers, it can fold a
> > > > > null pointer to integer literal directly.
> > > > >
> > > > > The above code does that, e.g.
> > > > >
> > > > > ```
> > > > > void test_cast_null_pointer_to_sizet_calee(size_t arg_private,
> > > > >size_t arg_local,
> > > > >size_t arg_global,
> > > > >size_t arg_constant,
> > > > >size_t arg_generic);
> > > > >
> > > > > // CHECK-LABEL: test_cast_null_pointer_to_sizet
> > > > > // CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 -1,
> > > > > i64 -1, i64 0, i64 0, i64 0)
> > > > > void test_cast_null_pointer_to_sizet(void) {
> > > > > test_cast_null_pointer_to_sizet_calee((size_t)((private char*)0),
> > > > > (size_t)((local char*)0),
> > > > > (size_t)((global char*)0),
> > > > > (size_t)((constant char*)0),
> > > > > (size_t)((generic char*)0));
> > > > > }
> > > > >
> > > > > ```
> > > > >
> > > > > Without the above code, we only get ptrtoint instructions.
> > > > Oh, does the constant folder not know how to fold ptrtoint(inttoptr(X))
> > > > -> X? I guess that's probably LLVM's nominal target-independence
> > > > rearing its head.
> > > >
> > > > Is getting a slightly LLVM constant actually important here? I would
> > > > prefer to avoid this complexity (and unnecessary recursive walk) if
> > > > possible.
> > > Since it's target dependent, it won't be constant folded by the target
> > > agnostic passes until very late in the backend, which may lose
> > > optimization opportunities. I think it's better folded by FE like other
> > > constants.
> > Are you sure? All of my attempts to produce these constants in LLVM seem
> > to get instantly folded, even without a target set in the IR:
> > i32 ptrtoint (i8* inttoptr (i32 -1 to i8*) to i32)
> > i64 ptrtoint (i8* inttoptr (i64 -1 to i8*) to i64)
> > This folding literally occurs within ConstantExpr::getPtrToInt, without any
> > passes running, and it seems to happen regardless of the pointer having an
> > addrspace.
> >
> > I suspect that the problem is that you additionally have an addrspacecast
> > constant in place, which almost certainly does block the constant folder.
> > The right fix for that is to ensure that your target's implementation of
> > performAddrSpaceCast makes an effort to peephole casts of Constants.
> In amdgpu target, null pointer in addr space 0 is represented as
> `addrspacecast int addrspace(4)* null to int*` instead of `inttoptr i32 -1 to
> i8*`. As addrspacecast is opaque to target-independent LLVM passes, they
> don't know how to fold it. Only the backend knows how to fold it.
>
> This could be a general situation, i.e., the non-zero null pointer is
> represented in some target specific form which LLVM does not know how to fold.
LLVM has deeply-baked-in assumptions that null is a zero bit pattern. It is
really, really not a good idea to assume that LLVM will never make any
assumptions about the behavior of addrspacecasts in its target-independent code.
Also, it is almost certainly possible to validly produce that constant with the
expectation that it will have value 0, and thus you special-case lowering in
the backend would be a miscompile.
I understand that you probably got into this situation by trying to give null a
non-zero representation in the backend and then slowly realized that you needed
frontend changes to avoid miscompiles. That is totally understandable. Please
understand that what are you actually doing now with this patch is
fundamentally changing where you lower null pointers, so that it's now only the
frontend which knows about the representation of null pointers, and the backend
just treats ConstantPointerNull as a special way of spelling the valid,
semantically non-null zero representation of a pointer.
https://reviews.llvm.org/D26196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl updated the summary for this revision.
yaxunl updated this revision to Diff 80804.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Revised by John's comments.
Added source QualType parameter to performAddrSpaceCast.
Changed linkage of tentative definition of global var with non-zero initializer
to weak linkage since common linkage requires zero initializer.
https://reviews.llvm.org/D26196
Files:
include/clang/AST/APValue.h
include/clang/AST/ASTContext.h
include/clang/Basic/TargetInfo.h
lib/AST/APValue.cpp
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/Basic/Targets.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/CodeGenTypes.h
lib/CodeGen/TargetInfo.cpp
lib/CodeGen/TargetInfo.h
test/CodeGenOpenCL/amdgpu-nullptr.cl
Index: test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -0,0 +1,534 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -emit-llvm -o - | FileCheck --check-prefix=NOOPT %s
+
+typedef struct {
+ private char *p1;
+ local char *p2;
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy1;
+
+typedef struct {
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy2;
+
+// LLVM requests global variable with common linkage to be initialized with zeroinitializer, therefore use -fno-common
+// to suppress common linkage for tentative definition.
+
+// Test 0 as initializer.
+
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p = 0;
+
+// CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p = 0;
+
+// CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p = 0;
+
+// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p = 0;
+
+// CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p = 0;
+
+// Test NULL as initializer.
+
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p_NULL = NULL;
+
+// CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p_NULL = NULL;
+
+// CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p_NULL = NULL;
+
+// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p_NULL = NULL;
+
+// CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p_NULL = NULL;
+
+// Test constant folding of null pointer.
+// A null pointer should be folded to a null pointer in the target address space.
+
+// CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 addrspace(4)* null, align 4
+generic int *fold_generic = (global int*)(generic float*)(private char*)0;
+
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* addrspacecast (i16 addrspace(4)* null to i16*), align 4
+private short *fold_priv = (private short*)(generic int*)(global void*)0;
+
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* inttoptr (i32 9 to i8*), align 4
+private char *fold_priv_arith = (private char*)0 + 10;
+
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
+int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
+
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
+int fold_int2 = (int) ((private void*)0 + 13);
+
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
+int fold_int3 = (int) ((private int*)0);
+
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
+int fold_int4 = (int) &((private int*)0)[2];
+
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
+int fold_int5 = (int) &((private StructTy1*)0)->p2;
+
+// Test static variable initialization.
+
+// NOOPT: @test_static_var.sp1 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp2 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp3 = internal addrspace(1)
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1532
+ return llvm::ConstantInt::get(ConvertType(DestTy),
+ CGF.getContext().getTargetNullPtrValue(E->getType()));
assert(!DestTy->isBooleanType() && "bool should use PointerToBool");
yaxunl wrote:
> rjmccall wrote:
> > yaxunl wrote:
> > > rjmccall wrote:
> > > > Why is this necessary? ptrtoint on the recursively-emitted null
> > > > pointer should do this automatically.
> > > Since the target knows the value in the null pointers, it can fold a null
> > > pointer to integer literal directly.
> > >
> > > The above code does that, e.g.
> > >
> > > ```
> > > void test_cast_null_pointer_to_sizet_calee(size_t arg_private,
> > >size_t arg_local,
> > >size_t arg_global,
> > >size_t arg_constant,
> > >size_t arg_generic);
> > >
> > > // CHECK-LABEL: test_cast_null_pointer_to_sizet
> > > // CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 -1, i64
> > > -1, i64 0, i64 0, i64 0)
> > > void test_cast_null_pointer_to_sizet(void) {
> > > test_cast_null_pointer_to_sizet_calee((size_t)((private char*)0),
> > > (size_t)((local char*)0),
> > > (size_t)((global char*)0),
> > > (size_t)((constant char*)0),
> > > (size_t)((generic char*)0));
> > > }
> > >
> > > ```
> > >
> > > Without the above code, we only get ptrtoint instructions.
> > Oh, does the constant folder not know how to fold ptrtoint(inttoptr(X)) ->
> > X? I guess that's probably LLVM's nominal target-independence rearing its
> > head.
> >
> > Is getting a slightly LLVM constant actually important here? I would
> > prefer to avoid this complexity (and unnecessary recursive walk) if
> > possible.
> Since it's target dependent, it won't be constant folded by the target
> agnostic passes until very late in the backend, which may lose optimization
> opportunities. I think it's better folded by FE like other constants.
Are you sure? All of my attempts to produce these constants in LLVM seem to
get instantly folded, even without a target set in the IR:
i32 ptrtoint (i8* inttoptr (i32 -1 to i8*) to i32)
i64 ptrtoint (i8* inttoptr (i64 -1 to i8*) to i64)
This folding literally occurs within ConstantExpr::getPtrToInt, without any
passes running, and it seems to happen regardless of the pointer having an
addrspace.
I suspect that the problem is that you additionally have an addrspacecast
constant in place, which almost certainly does block the constant folder. The
right fix for that is to ensure that your target's implementation of
performAddrSpaceCast makes an effort to peephole casts of Constants.
Comment at: lib/CodeGen/TargetInfo.h:236
+ virtual llvm::Value *performAddrSpaceCast(CodeGen::CodeGenFunction ,
+ Expr *E, QualType DestTy) const;
+
rjmccall wrote:
> This should just take a Value* and the source and dest types.
Please also pass the source QualType. The QualType is necessary to determine
the source-language address space, which is how an implementation can correctly
determine the null-pointer representation in the source type.
https://reviews.llvm.org/D26196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl updated the summary for this revision.
yaxunl updated this revision to Diff 80600.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Revised by John's comments.
Fixed typos.
Changed parameter of performAddrSpaceCast.
Fixed constant folding of ptrtoint with side effect and added a test.
https://reviews.llvm.org/D26196
Files:
include/clang/AST/APValue.h
include/clang/AST/ASTContext.h
include/clang/Basic/TargetInfo.h
lib/AST/APValue.cpp
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/Basic/Targets.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/CodeGenTypes.h
lib/CodeGen/TargetInfo.cpp
lib/CodeGen/TargetInfo.h
test/CodeGenOpenCL/amdgpu-nullptr.cl
Index: test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -0,0 +1,530 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck --check-prefix=NOOPT %s
+
+typedef struct {
+ private char *p1;
+ local char *p2;
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy1;
+
+typedef struct {
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy2;
+
+// LLVM requests global variable with common linkage to be initialized with zeroinitializer, therefore use -fno-common
+// to suppress common linkage for tentative definition.
+
+// Test 0 as initializer.
+
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p = 0;
+
+// CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p = 0;
+
+// CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p = 0;
+
+// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p = 0;
+
+// CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p = 0;
+
+// Test NULL as initializer.
+
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p_NULL = NULL;
+
+// CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p_NULL = NULL;
+
+// CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p_NULL = NULL;
+
+// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p_NULL = NULL;
+
+// CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p_NULL = NULL;
+
+// Test constant folding of null pointer.
+// A null pointer should be folded to a null pointer in the target address space.
+
+// CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 addrspace(4)* null, align 4
+generic int *fold_generic = (global int*)(generic float*)(private char*)0;
+
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* addrspacecast (i16 addrspace(4)* null to i16*), align 4
+private short *fold_priv = (private short*)(generic int*)(global void*)0;
+
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* inttoptr (i32 9 to i8*), align 4
+private char *fold_priv_arith = (private char*)0 + 10;
+
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
+int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
+
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
+int fold_int2 = (int) ((private void*)0 + 13);
+
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
+int fold_int3 = (int) ((private int*)0);
+
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
+int fold_int4 = (int) &((private int*)0)[2];
+
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
+int fold_int5 = (int) &((private StructTy1*)0)->p2;
+
+// Test static variable initialization.
+
+// NOOPT: @test_static_var.sp1 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp2 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp3 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT:
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl marked 4 inline comments as done.
yaxunl added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1532
+ return llvm::ConstantInt::get(ConvertType(DestTy),
+ CGF.getContext().getTargetNullPtrValue(E->getType()));
assert(!DestTy->isBooleanType() && "bool should use PointerToBool");
rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > Why is this necessary? ptrtoint on the recursively-emitted null pointer
> > > should do this automatically.
> > Since the target knows the value in the null pointers, it can fold a null
> > pointer to integer literal directly.
> >
> > The above code does that, e.g.
> >
> > ```
> > void test_cast_null_pointer_to_sizet_calee(size_t arg_private,
> >size_t arg_local,
> >size_t arg_global,
> >size_t arg_constant,
> >size_t arg_generic);
> >
> > // CHECK-LABEL: test_cast_null_pointer_to_sizet
> > // CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 -1, i64 -1,
> > i64 0, i64 0, i64 0)
> > void test_cast_null_pointer_to_sizet(void) {
> > test_cast_null_pointer_to_sizet_calee((size_t)((private char*)0),
> > (size_t)((local char*)0),
> > (size_t)((global char*)0),
> > (size_t)((constant char*)0),
> > (size_t)((generic char*)0));
> > }
> >
> > ```
> >
> > Without the above code, we only get ptrtoint instructions.
> Oh, does the constant folder not know how to fold ptrtoint(inttoptr(X)) -> X?
> I guess that's probably LLVM's nominal target-independence rearing its head.
>
> Is getting a slightly LLVM constant actually important here? I would prefer
> to avoid this complexity (and unnecessary recursive walk) if possible.
Since it's target dependent, it won't be constant folded by the target agnostic
passes until very late in the backend, which may lose optimization
opportunities. I think it's better folded by FE like other constants.
https://reviews.llvm.org/D26196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
rjmccall added inline comments.
Comment at: include/clang/AST/APValue.h:379
void setLValue(LValueBase B, const CharUnits , NoLValuePath,
- unsigned CallIndex);
+ unsigned CallIndex, bool IsNuppPtr);
void setLValue(LValueBase B, const CharUnits ,
Typo.
Comment at: lib/AST/ExprConstant.cpp:1153
+IsNullPtr = false;
+}
+void adjustOffsetAndIndex(EvalInfo , const Expr *E, uint64_t Index,
Hmm. I think the code would be clearer overall if this were conditionalized in
the callers instead of here.
Comment at: lib/CodeGen/CGExprScalar.cpp:1532
+ return llvm::ConstantInt::get(ConvertType(DestTy),
+ CGF.getContext().getTargetNullPtrValue(E->getType()));
assert(!DestTy->isBooleanType() && "bool should use PointerToBool");
yaxunl wrote:
> rjmccall wrote:
> > Why is this necessary? ptrtoint on the recursively-emitted null pointer
> > should do this automatically.
> Since the target knows the value in the null pointers, it can fold a null
> pointer to integer literal directly.
>
> The above code does that, e.g.
>
> ```
> void test_cast_null_pointer_to_sizet_calee(size_t arg_private,
>size_t arg_local,
>size_t arg_global,
>size_t arg_constant,
>size_t arg_generic);
>
> // CHECK-LABEL: test_cast_null_pointer_to_sizet
> // CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 -1, i64 -1,
> i64 0, i64 0, i64 0)
> void test_cast_null_pointer_to_sizet(void) {
> test_cast_null_pointer_to_sizet_calee((size_t)((private char*)0),
> (size_t)((local char*)0),
> (size_t)((global char*)0),
> (size_t)((constant char*)0),
> (size_t)((generic char*)0));
> }
>
> ```
>
> Without the above code, we only get ptrtoint instructions.
Oh, does the constant folder not know how to fold ptrtoint(inttoptr(X)) -> X?
I guess that's probably LLVM's nominal target-independence rearing its head.
Is getting a slightly LLVM constant actually important here? I would prefer to
avoid this complexity (and unnecessary recursive walk) if possible.
Comment at: lib/CodeGen/TargetInfo.h:236
+ virtual llvm::Value *performAddrSpaceCast(CodeGen::CodeGenFunction ,
+ Expr *E, QualType DestTy) const;
+
This should just take a Value* and the source and dest types.
https://reviews.llvm.org/D26196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl updated this revision to Diff 80488.
yaxunl added a comment.
Fix some missed function name and remove accidentally added #if 0.
https://reviews.llvm.org/D26196
Files:
include/clang/AST/APValue.h
include/clang/AST/ASTContext.h
include/clang/Basic/TargetInfo.h
lib/AST/APValue.cpp
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/Basic/Targets.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/CodeGenTypes.h
lib/CodeGen/TargetInfo.cpp
lib/CodeGen/TargetInfo.h
test/CodeGenOpenCL/amdgpu-nullptr.cl
Index: test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -0,0 +1,529 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck --check-prefix=NOOPT %s
+
+typedef struct {
+ private char *p1;
+ local char *p2;
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy1;
+
+typedef struct {
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy2;
+
+// LLVM requests global variable with common linkage to be initialized with zeroinitializer, therefore use -fno-common
+// to suppress common linkage for tentative definition.
+
+// Test 0 as initializer.
+
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p = 0;
+
+// CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p = 0;
+
+// CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p = 0;
+
+// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p = 0;
+
+// CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p = 0;
+
+// Test NULL as initializer.
+
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p_NULL = NULL;
+
+// CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p_NULL = NULL;
+
+// CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p_NULL = NULL;
+
+// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p_NULL = NULL;
+
+// CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p_NULL = NULL;
+
+// Test constant folding of null pointer.
+// A null pointer should be folded to a null pointer in the target address space.
+
+// CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 addrspace(4)* null, align 4
+generic int *fold_generic = (global int*)(generic float*)(private char*)0;
+
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* addrspacecast (i16 addrspace(4)* null to i16*), align 4
+private short *fold_priv = (private short*)(generic int*)(global void*)0;
+
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* inttoptr (i32 9 to i8*), align 4
+private char *fold_priv_arith = (private char*)0 + 10;
+
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
+int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
+
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
+int fold_int2 = (int) ((private void*)0 + 13);
+
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
+int fold_int3 = (int) ((private int*)0);
+
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
+int fold_int4 = (int) &((private int*)0)[2];
+
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
+int fold_int5 = (int) &((private StructTy1*)0)->p2;
+
+// Test static variable initialization.
+
+// NOOPT: @test_static_var.sp1 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp2 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp3 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp4 = internal addrspace(1) global i8* null, align 4
+// NOOPT: @test_static_var.sp5 = internal addrspace(1) global i8* null, align 4
+// NOOPT: @test_static_var.SS1
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl updated this revision to Diff 80485.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Revised by John's comments.
Changed Ptr in function names to Pointer.
Added TargetCodeGenInfo::performAddrSpaceCast.
Fixed folding addrspacecast of null pointer with side effect and added a test.
https://reviews.llvm.org/D26196
Files:
include/clang/AST/APValue.h
include/clang/AST/ASTContext.h
include/clang/Basic/TargetInfo.h
lib/AST/APValue.cpp
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/Basic/Targets.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/CodeGenTypes.h
lib/CodeGen/TargetInfo.cpp
lib/CodeGen/TargetInfo.h
test/CodeGenOpenCL/amdgpu-nullptr.cl
Index: test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -0,0 +1,529 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck --check-prefix=NOOPT %s
+
+typedef struct {
+ private char *p1;
+ local char *p2;
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy1;
+
+typedef struct {
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy2;
+
+// LLVM requests global variable with common linkage to be initialized with zeroinitializer, therefore use -fno-common
+// to suppress common linkage for tentative definition.
+
+// Test 0 as initializer.
+
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p = 0;
+
+// CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p = 0;
+
+// CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p = 0;
+
+// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p = 0;
+
+// CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p = 0;
+
+// Test NULL as initializer.
+
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p_NULL = NULL;
+
+// CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p_NULL = NULL;
+
+// CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p_NULL = NULL;
+
+// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p_NULL = NULL;
+
+// CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p_NULL = NULL;
+
+// Test constant folding of null pointer.
+// A null pointer should be folded to a null pointer in the target address space.
+
+// CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 addrspace(4)* null, align 4
+generic int *fold_generic = (global int*)(generic float*)(private char*)0;
+
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* addrspacecast (i16 addrspace(4)* null to i16*), align 4
+private short *fold_priv = (private short*)(generic int*)(global void*)0;
+
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* inttoptr (i32 9 to i8*), align 4
+private char *fold_priv_arith = (private char*)0 + 10;
+
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
+int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
+
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
+int fold_int2 = (int) ((private void*)0 + 13);
+
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
+int fold_int3 = (int) ((private int*)0);
+
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
+int fold_int4 = (int) &((private int*)0)[2];
+
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
+int fold_int5 = (int) &((private StructTy1*)0)->p2;
+
+// Test static variable initialization.
+
+// NOOPT: @test_static_var.sp1 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp2 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp3 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT:
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl marked 7 inline comments as done.
yaxunl added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1532
+ return llvm::ConstantInt::get(ConvertType(DestTy),
+ CGF.getContext().getTargetNullPtrValue(E->getType()));
assert(!DestTy->isBooleanType() && "bool should use PointerToBool");
rjmccall wrote:
> Why is this necessary? ptrtoint on the recursively-emitted null pointer
> should do this automatically.
Since the target knows the value in the null pointers, it can fold a null
pointer to integer literal directly.
The above code does that, e.g.
```
void test_cast_null_pointer_to_sizet_calee(size_t arg_private,
size_t arg_local,
size_t arg_global,
size_t arg_constant,
size_t arg_generic);
// CHECK-LABEL: test_cast_null_pointer_to_sizet
// CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 -1, i64 -1, i64
0, i64 0, i64 0)
void test_cast_null_pointer_to_sizet(void) {
test_cast_null_pointer_to_sizet_calee((size_t)((private char*)0),
(size_t)((local char*)0),
(size_t)((global char*)0),
(size_t)((constant char*)0),
(size_t)((generic char*)0));
}
```
Without the above code, we only get ptrtoint instructions.
https://reviews.llvm.org/D26196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
rjmccall added inline comments.
Comment at: include/clang/Basic/TargetInfo.h:306
+ /// \param AddrSpace address space of pointee in source language.
+ virtual uint64_t getNullPtrValue(unsigned AddrSpace) const {
+return 0;
We normally spell out "Pointer", I think.
Comment at: lib/AST/APValue.cpp:585
+bool APValue::isNullPtr() const {
+ return isLValue() && ((const LV*)(const char*)Data.buffer)->IsNullPtr;
+}
Seems reasonable to assert isLValue() here.
Comment at: lib/CodeGen/CGExprAgg.cpp:1054
+return ICE->getCastKind() == CK_NullToPointer &&
+CGF.getTypes().isZeroInitializable(E->getType());
// '\0'
This is probably a common enough case that it's worth "inlining", i.e. adding a
isPointerZeroInitializable() method that asserts that it's been given a pointer
type and does the right thing with it.
Comment at: lib/CodeGen/CGExprScalar.cpp:1407
return Builder.CreatePointerBitCastOrAddrSpaceCast(Src,
ConvertType(DestTy));
}
As discussed, please also delegate addrspace conversions into
TargetCodeGenInfo. You don't have to implement them for your target if you
don't want, but please do the delegation correctly.
Also, even if you decide not to implement the actual dynamic comparison logic,
you should at least teach your target's implementation to recognize when the
original pointer is a constant null pointer and just produce the correct new
constant. That should eliminate the need for this special case here, as well
as fixing the case where E is a null pointer with side effects like (foo(),
NULL).
Comment at: lib/CodeGen/CGExprScalar.cpp:1532
+ return llvm::ConstantInt::get(ConvertType(DestTy),
+ CGF.getContext().getTargetNullPtrValue(E->getType()));
assert(!DestTy->isBooleanType() && "bool should use PointerToBool");
Why is this necessary? ptrtoint on the recursively-emitted null pointer should
do this automatically.
Comment at: lib/CodeGen/CodeGenModule.h:1156
+ /// Get target specific null pointer.
+ llvm::Constant *getNullPtr(llvm::PointerType *T, QualType QT);
+
Like above, this should be getNullPointer. Also, please document which type QT
is: the pointer type or the pointee type.
Comment at: lib/CodeGen/TargetInfo.h:228
+ virtual llvm::Constant *getNullPtr(const CodeGen::CodeGenModule ,
+ llvm::PointerType *T, QualType QT) const;
+
Same as the comment on getNullPtr above.
https://reviews.llvm.org/D26196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl added a comment. ping https://reviews.llvm.org/D26196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl updated the summary for this revision.
yaxunl updated this revision to Diff 79406.
yaxunl added a comment.
Revised by John's comments.
Emit null pointer in target-specific representation in folded constant
expressions.
Fix casting null pointer to integer in constant expressions.
https://reviews.llvm.org/D26196
Files:
include/clang/AST/APValue.h
include/clang/AST/ASTContext.h
include/clang/Basic/TargetInfo.h
lib/AST/APValue.cpp
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/Basic/Targets.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/TargetInfo.cpp
lib/CodeGen/TargetInfo.h
test/CodeGenOpenCL/amdgpu-nullptr.cl
Index: test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -0,0 +1,527 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -cl-std=CL2.0 -include opencl-c.h -triple amdgcn -fno-common -emit-llvm -o - | FileCheck --check-prefix=NOOPT %s
+
+typedef struct {
+ private char *p1;
+ local char *p2;
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy1;
+
+typedef struct {
+ constant char *p3;
+ global char *p4;
+ generic char *p5;
+} StructTy2;
+
+// LLVM requests global variable with common linkage to be initialized with zeroinitializer, therefore use -fno-common
+// to suppress common linkage for tentative definition.
+
+// Test 0 as initializer.
+
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p = 0;
+
+// CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p = 0;
+
+// CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p = 0;
+
+// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p = 0;
+
+// CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p = 0;
+
+// Test NULL as initializer.
+
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+private char *private_p_NULL = NULL;
+
+// CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
+local char *local_p_NULL = NULL;
+
+// CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 4
+global char *global_p_NULL = NULL;
+
+// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 4
+constant char *constant_p_NULL = NULL;
+
+// CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 4
+generic char *generic_p_NULL = NULL;
+
+// Test constant folding of null pointer.
+// A null pointer should be folded to a null pointer in the target address space.
+
+// CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 addrspace(4)* null, align 4
+generic int *fold_generic = (global int*)(generic float*)(private char*)0;
+
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* addrspacecast (i16 addrspace(4)* null to i16*), align 4
+private short *fold_priv = (private short*)(generic int*)(global void*)0;
+
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* inttoptr (i32 9 to i8*), align 4
+private char *fold_priv_arith = (private char*)0 + 10;
+
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
+int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
+
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
+int fold_int2 = (int) ((private void*)0 + 13);
+
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
+int fold_int3 = (int) ((private int*)0);
+
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
+int fold_int4 = (int) &((private int*)0)[2];
+
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
+int fold_int5 = (int) &((private StructTy1*)0)->p2;
+
+// Test static variable initialization.
+
+// NOOPT: @test_static_var.sp1 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp2 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp3 = internal addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// NOOPT: @test_static_var.sp4 = internal addrspace(1) global i8*
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
rjmccall added a comment.
In https://reviews.llvm.org/D26196#602660, @yaxunl wrote:
> > But if you do need to support these conversions for some reason, the
> > correct behavior is to ensure that null is mapped to null.
>
> I only need to do this for constant folding, right? I found that the current
> implementation is already able to cast null pointer to the correct
> representation of null pointer in another address space for constant
> expression. e.g. file-scope variables
>
> generic char *gen = (global char*)(generic char*)(private char*)0;
> private char *priv = (private char*)(generic char*)(global char*)0;
> global char *glob = (global char*)(generic char*)(global char*)0;
>
>
>
>
> are translated to
>
> @gen = addrspace(1) global i8 addrspace(4)* null, align 4
> @priv = addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to
> i8*), align 4
> @glob = addrspace(1) global i8 addrspace(1)* null, align 4
>
>
> This is because null pointers are handled in APValue and
> PointerExprEvaluator::VisitCastExpr. Once a null pointer is identified, it
> can survive passing through addrspacecast and bitcast. When it gets folded,
> it becomes the target-specific null pointer representation in the target
> address space.
>
> However, if an addrspacecast is not going through constant folding, it will
> be translated to LLVM addrspacecast, e.g.
>
> void test_cast(void) {
> global char* glob = (global char*)(generic char*)0;
>}
>
>
> Since the initializer of the function-scope variable does not go through
> constant folding, it is not translated to target-specific null pointer
> representation in the target address space. Instead, it is simply an
> addrspacecast of the original null pointer:
>
> define void @test_cast() #0 {
> entry:
> %glob = alloca i8 addrspace(1)*, align 4
> store i8 addrspace(1)* addrspacecast (i8 addrspace(4)* null to i8
> addrspace(1)*), i8 addrspace(1)** %glob, align 4
> ret void
> }
>
>
> This is still correct translation, since backend should be able to understand
> addrspacecast of null pointer.
>
> Do you think in the last case I should try to fold the addrspacecast?
Yes. I think it would be wrong for an addrspacecast of a literal
llvm::ConstantPointerNull to have different semantics from an addrspacecast of
an opaque value that happens to be an llvm::ConstantPointerNull.
https://reviews.llvm.org/D26196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl marked 3 inline comments as done.
yaxunl added a comment.
> But if you do need to support these conversions for some reason, the correct
> behavior is to ensure that null is mapped to null.
I only need to do this for constant folding, right? I found that the current
implementation is already able to cast null pointer to the correct
representation of null pointer in another address space for constant
expression. e.g. file-scope variables
generic char *gen = (global char*)(generic char*)(private char*)0;
private char *priv = (private char*)(generic char*)(global char*)0;
global char *glob = (global char*)(generic char*)(global char*)0;
are translated to
@gen = addrspace(1) global i8 addrspace(4)* null, align 4
@priv = addrspace(1) global i8* addrspacecast (i8 addrspace(4)* null to i8*),
align 4
@glob = addrspace(1) global i8 addrspace(1)* null, align 4
This is because null pointers are handled in APValue and
PointerExprEvaluator::VisitCastExpr. Once a null pointer is identified, it can
survive passing through addrspacecast and bitcast. When it gets folded, it
becomes the target-specific null pointer representation in the target address
space.
However, if an addrspacecast is not going through constant folding, it will be
translated to LLVM addrspacecast, e.g.
void test_cast(void) {
global char* glob = (global char*)(generic char*)0;
}
Since the initializer of the function-scope variable does not go through
constant folding, it is not translated to target-specific null pointer
representation in the target address space. Instead, it is simply an
addrspacecast of the original null pointer:
define void @test_cast() #0 {
entry:
%glob = alloca i8 addrspace(1)*, align 4
store i8 addrspace(1)* addrspacecast (i8 addrspace(4)* null to i8
addrspace(1)*), i8 addrspace(1)** %glob, align 4
ret void
}
This is still correct translation, since backend should be able to understand
addrspacecast of null pointer.
Do you think in the last case I should try to fold the addrspacecast?
Thanks.
https://reviews.llvm.org/D26196
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
rjmccall added a comment. In https://reviews.llvm.org/D26196#593741, @yaxunl wrote: > >> It seems the casting from a pointer to different address space is not > >> affected by this change. When a null pointer is casted to different > >> address space, it is casted the same way as an ordinary pointer, e.g. by > >> addrspacecast. > > > > You mean, the code-generation for that knows about your special null > > pointer representation? That is confusing. > > Do you mean if a null pointer in one address space is casted to another > address space, we should use the specific null pointer representation in the > new address space, instead of a simple addrspacecast? That's the general user expectation, yes: a null pointer should be converted to a null pointer. Technically, whether this required is up to the appropriate language standard. The standard rules for address spaces are laid out by Embedded C (ISO/IEC TR 18037), which has this to say in 5.1.3p4: > A non-null pointer into an address space A can be cast to a pointer into > another address space B, but such a cast is undefined if the source pointer > does not point to a location in B. Note that if A is a subset of B, the cast > is always valid; however, if B is a subset of A, the cast is valid only if > the source pointer refers to a location in B. A null pointer into one address > space can be cast to a null pointer into any overlapping address space. The wording could be better, but I think this is fairly clearly saying that you do need to map null to null. Now, my understanding is that you're not actually implementing Embedded C, you're implementing Open CL. The Open CL specification that I have on hand (v1.1) says that __global, __local, __constant, and __private are disjoint and that casts between address spaces are illegal. So this might not actually matter to you. But if you do need to support these conversions for some reason, the correct behavior is to ensure that null is mapped to null. https://reviews.llvm.org/D26196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl added a comment. >> It seems the casting from a pointer to different address space is not >> affected by this change. When a null pointer is casted to different address >> space, it is casted the same way as an ordinary pointer, e.g. by >> addrspacecast. > > You mean, the code-generation for that knows about your special null pointer > representation? That is confusing. Do you mean if a null pointer in one address space is casted to another address space, we should use the specific null pointer representation in the new address space, instead of a simple addrspacecast? https://reviews.llvm.org/D26196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl added a comment. >> It seems the casting from a pointer to different address space is not >> affected by this change. When a null pointer is casted to different address >> space, it is casted the same way as an ordinary pointer, e.g. by >> addrspacecast. > > You mean, the code-generation for that knows about your special null pointer > representation? That is confusing. The null pointer is represented in a new way, but the new representation has the same type as before. For example, a null pointer in addr space 0 used to be `i8 *null`, now it becomes `addrspacecast i8 addrspace(4)* null to i8*`. It is still of type i8*, so nothing is changed about casting a pointer to a different address space. https://reviews.llvm.org/D26196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
rjmccall added a comment. In https://reviews.llvm.org/D26196#592823, @yaxunl wrote: > Hi John, > > It seems the casting from a pointer to different address space is not > affected by this change. When a null pointer is casted to different address > space, it is casted the same way as an ordinary pointer, e.g. by > addrspacecast. You mean, the code-generation for that knows about your special null pointer representation? That is confusing. https://reviews.llvm.org/D26196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26196: Add support for non-zero null pointer for C and OpenCL
yaxunl added a comment. Hi John, It seems the casting from a pointer to different address space is not affected by this change. When a null pointer is casted to different address space, it is casted the same way as an ordinary pointer, e.g. by addrspacecast. I think this patch covers most of the changes that is needed for supporting non-zero null pointer in C and OpenCL. Is there anything else missing? Thanks. https://reviews.llvm.org/D26196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
