Anastasia added inline comments.
================ Comment at: include/clang/AST/ASTContext.h:2328 + return AddrSpaceMapMangling || + AS >= LangAS::target_first; } ---------------- Anastasia wrote: > yaxunl wrote: > > Anastasia wrote: > > > So we couldn't use the LangAS::Count instead? > > > > > > I have the same comment in other places that use LangAS::target_first. > > > Why couldn't we simply use LangAS::Count? It there any point in having > > > two tags? > > > > > > Another comment is why do we need ASes specified by > > > `__attribute__((address_space(n)))` to be unique enum number at the end > > > of named ASes of OpenCL and CUDA? I think conceptually the full range of > > > ASes can be used in C because the ASes from OpenCL and CUDA are not > > > available there anyways. > > I will use LangAS::Count instead and remove target_first, since their > > values are the same. > > > > For your second question: the values for > > `__attribute__((address_space(n)))` need to be different from the language > > specific address space values because they are mapped to target address > > space differently. > > > > For language specific address space, they are mapped through a target > > defined mapping table. > > > > For `__attribute__((address_space(n)))`, the target address space should be > > the same as n, without going through the mapping table. > > > > If they are defined in overlapping value ranges, they cannot be handled in > > different ways. > > > > > Target address space map currently corresponds to the named address spaces of > OpenCL and CUDA only. So if the idea is to avoid overlapping with those we > should extend the table? Although, I don't see how this can be done because > it will require fixed semantic of address spaces in C which is currently > undefined. Perhaps I am missing something but I don't see any change here that makes non-named address spaces follow different path for the target. Also does this change relate to alloca extension in some way? I still struggle to understand this fully... All I can see is that this change restricts overlapping of named and non-named address spaces but I can't clearly understand the motivation for this. ================ Comment at: lib/Basic/Targets.cpp:2035 static const LangAS::Map AMDGPUPrivateIsZeroMap = { + 4, // Default 1, // opencl_global ---------------- This should use address space attribute 4 for all non-AS type objects. Could we make sure we have a codegen test for this? https://reviews.llvm.org/D31404 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits