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
test_numeric_as_to_generic_explicit_cast(__attribute__((address_space(3)))
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
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
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
-
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
> i
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 ne
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
li
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. (HCC),
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
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 a
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 we
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
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:
> >
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
lib/Basic
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:
> > An
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
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 if
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
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 suppor
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:
> > An
20 matches
Mail list logo