[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-04 Thread John McCall via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D26196#587135, @yaxunl wrote: > In https://reviews.llvm.org/D26196#586433, @rjmccall wrote: > > > I think there are a number of conceptual questions here, chiefly among them > > whether an llvm::ConstantPointerNull in an address-space type

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-03 Thread Matt Arsenault via cfe-commits
arsenm added a comment. In https://reviews.llvm.org/D26196#586433, @rjmccall wrote: > I think there are a number of conceptual questions here, chiefly among them > whether an llvm::ConstantPointerNull in an address-space type should have the > correct representation for the target. If the

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-03 Thread Tom Stellard via cfe-commits
tstellarAMD added a comment. In https://reviews.llvm.org/D26196#586723, @Anastasia wrote: > My understanding of NULL constant in IR was that it doesn't assume any > specific integer value (i.e. 0). And currently as I can see it is lowered > very late in LLVM backend during the code selection

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-03 Thread Anastasia Stulova via cfe-commits
Anastasia added a comment. My understanding of NULL constant in IR was that it doesn't assume any specific integer value (i.e. 0). And currently as I can see it is lowered very late in LLVM backend during the code selection phase. Looking at the lowering code in LLVM, it seems like it

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-02 Thread John McCall via cfe-commits
rjmccall added a comment. I think there are a number of conceptual questions here, chiefly among them whether an llvm::ConstantPointerNull in an address-space type should have the correct representation for the target. If the decision for that is "no", then ok, we can produce an explicit

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-02 Thread Yaxun Liu via cfe-commits
yaxunl added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:7039 + if (CGM.getTarget().getTriple().getArch() != llvm::Triple::amdgcn || +(AS != Ctx.getTargetAddressSpace(LangAS::opencl_local) && AS != 0)) +return C; arsenm wrote: > Shouldn't

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-02 Thread Yaxun Liu via cfe-commits
yaxunl updated the summary for this revision. yaxunl updated this revision to Diff 76792. yaxunl added a comment. Fixed translation of a pointer to bool. Added more tests. https://reviews.llvm.org/D26196 Files: lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExprConstant.cpp

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-01 Thread Matt Arsenault via cfe-commits
arsenm added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:6957-6958 + + llvm::Constant *translateNullPtr(const CodeGen::CodeGenModule , + llvm::Constant *C) const override; }; I was thinking that addrspacecast (generic null) should be valid for

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-01 Thread Matt Arsenault via cfe-commits
arsenm added inline comments. Comment at: test/CodeGenOpenCL/amdgpu-nullptr.cl:30 + +// CHECK: icmp eq i8 addrspace(1)* %p, null +void cmp_global(global char* p) { Missing check-label https://reviews.llvm.org/D26196

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-01 Thread Matt Arsenault via cfe-commits
arsenm added inline comments. Comment at: test/CodeGenOpenCL/amdgpu-nullptr.cl:49 +} + I think there need to be a lot more tests. A few I can think of: - Tests that compare to NULL instead of 0 - Pointer compares without the explicit 0, i.e. if (p)/if(!p)

[PATCH] D26196: AMDGPU: Translate null pointers in private and local addr space

2016-11-01 Thread Yaxun Liu via cfe-commits
yaxunl created this revision. yaxunl added reviewers: arsenm, tstellarAMD, rjmccall. yaxunl added a subscriber: cfe-commits. Herald added subscribers: tony-tye, nhaehnle, wdng, kzhuravl. In amdgcn target, null pointers in global, constant, and generic address space take value 0 but null pointers