[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm closed this revision. arsenm added a comment. r338707 https://reviews.llvm.org/D47154 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47154: Try to make builtin address space declarations not useless

yaxunl accepted this revision. yaxunl added a comment. This revision is now accepted and ready to land. LGTM. Thanks. I missed the addr space casts you added to CodeGenFunction::EmitBuiltinExpr. With those casts it should work. For other downstream address space agnostic languages, e.g.

[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm updated this revision to Diff 157885. arsenm added a comment. Remove old run line https://reviews.llvm.org/D47154 Files: include/clang/AST/ASTContext.h include/clang/Basic/BuiltinsAMDGPU.def include/clang/Basic/TargetInfo.h lib/AST/ASTContext.cpp lib/Basic/Targets/AMDGPU.h

[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm added inline comments. Comment at: include/clang/Basic/TargetInfo.h:1157 + /// language address space. + virtual LangAS getCUDABuiltinAddressSpace(unsigned AS) const { +return getLangASFromTargetAS(AS); yaxunl wrote: > I think this function is not

[PATCH] D47154: Try to make builtin address space declarations not useless

yaxunl added a comment. In https://reviews.llvm.org/D47154#1108813, @tra wrote: > CUDA does not expose explicit AS on clang size. All pointers are treated as > generic and we infer specific address space only in LLVM. > `__nvvm_atom_*_[sg]_*` builtins should probably be removed as they are >

[PATCH] D47154: Try to make builtin address space declarations not useless

yaxunl added inline comments. Comment at: test/CodeGenOpenCL/builtins-amdgcn.cl:3 +// RUN: %clang_cc1 -cl-std=CL2.0 -triple amdgcn-unknown-unknown -S -emit-llvm -o - %s | FileCheck -enable-var-scope %s +// RUN: %clang_cc1 -cl-std=CL2.0 -triple amdgcn-unknown-unknown-opencl -S

[PATCH] D47154: Try to make builtin address space declarations not useless

yaxunl added inline comments. Comment at: include/clang/Basic/TargetInfo.h:1157 + /// language address space. + virtual LangAS getCUDABuiltinAddressSpace(unsigned AS) const { +return getLangASFromTargetAS(AS); I think this function is not needed. Although

[PATCH] D47154: Try to make builtin address space declarations not useless

yaxunl requested changes to this revision. yaxunl added inline comments. This revision now requires changes to proceed. Comment at: lib/Basic/Targets/AMDGPU.h:398 + + LangAS getCUDABuiltinAddressSpace(unsigned AS) const override { +return LangAS::Default; I

[PATCH] D47154: Try to make builtin address space declarations not useless

Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM from OpenCL side! Thanks! Comment at: test/SemaOpenCL/numbered-address-space.cl:9 +void

[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm added a comment. ping https://reviews.llvm.org/D47154 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:3500 +if (auto *PtrTy = dyn_cast(PTy)) { + if (PtrTy->getAddressSpace() != + ArgValue->getType()->getPointerAddressSpace()) { Anastasia wrote: > arsenm wrote: > >

[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm updated this revision to Diff 154561. arsenm added a comment. Add sema test for numbered address spaces https://reviews.llvm.org/D47154 Files: include/clang/AST/ASTContext.h include/clang/Basic/BuiltinsAMDGPU.def include/clang/Basic/TargetInfo.h lib/AST/ASTContext.cpp

[PATCH] D47154: Try to make builtin address space declarations not useless

Anastasia added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:3500 +if (auto *PtrTy = dyn_cast(PTy)) { + if (PtrTy->getAddressSpace() != + ArgValue->getType()->getPointerAddressSpace()) { arsenm wrote: > Anastasia wrote: >

[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:3500 +if (auto *PtrTy = dyn_cast(PTy)) { + if (PtrTy->getAddressSpace() != + ArgValue->getType()->getPointerAddressSpace()) { Anastasia wrote: > arsenm wrote: > >

[PATCH] D47154: Try to make builtin address space declarations not useless

Anastasia added inline comments. Comment at: include/clang/Basic/BuiltinsAMDGPU.def:49 + +// FIXME: Need to disallow constant address space. BUILTIN(__builtin_amdgcn_div_scale, "dddbb*", "n") arsenm wrote: > Anastasia wrote: > > Do you plan to provide the

[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm updated this revision to Diff 150179. arsenm added a comment. Rebase and add comment https://reviews.llvm.org/D47154 Files: include/clang/AST/ASTContext.h include/clang/Basic/BuiltinsAMDGPU.def include/clang/Basic/TargetInfo.h lib/AST/ASTContext.cpp lib/Basic/Targets/AMDGPU.h

[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm added inline comments. Comment at: include/clang/Basic/BuiltinsAMDGPU.def:49 + +// FIXME: Need to disallow constant address space. BUILTIN(__builtin_amdgcn_div_scale, "dddbb*", "n") Anastasia wrote: > Do you plan to provide the support for it later? Or

[PATCH] D47154: Try to make builtin address space declarations not useless

tra added a comment. CUDA does not expose explicit AS on clang size. All pointers are treated as generic and we infer specific address space only in LLVM. `__nvvm_atom_*_[sg]_*` builtins should probably be removed as they are indeed useless without pointers with explicit AS and NVCC itself does

[PATCH] D47154: Try to make builtin address space declarations not useless

Anastasia added inline comments. Comment at: include/clang/Basic/BuiltinsAMDGPU.def:49 + +// FIXME: Need to disallow constant address space. BUILTIN(__builtin_amdgcn_div_scale, "dddbb*", "n") Do you plan to provide the support for it later? Or if else perhaps

[PATCH] D47154: Try to make builtin address space declarations not useless

arsenm created this revision. arsenm added reviewers: Anastasia, yaxunl, rjmccall. Herald added subscribers: tpr, nhaehnle, wdng. The way address space declarations for builtins currently work is nearly useless. The code assumes the address spaces used for builtins is a confusingly named "target