Anastasia added a comment.

I can't see clearly why the alloca has to be extended to accommodate the 
address space too? Couldn't  the address space for alloca just be taken 
directly from the data layout?

In fact is seems from the LLVM review, an address space for alloca doesn't go 
into the bitcode.



================
Comment at: include/clang/Basic/AddressSpaces.h:28
 enum ID {
-  Offset = 0x7FFF00,
+  Default = 0,
 
----------------
Somehow I wish that opencl_private would be represented explicitly instead and 
then an absence of an address space attribute would signify the default one to 
be used. But since opencl_private has always been represented as an absence of 
an address space attribute not only in AST but in IR as well, I believe it 
might be a bigger change now. However, how does this default address space 
align with default AS we put during type parsing in processTypeAttrs 
(https://reviews.llvm.org/D13168). I think after this step we shouldn't need 
default AS explicitly any longer? 


================
Comment at: include/clang/Basic/AddressSpaces.h:41
+
+  target_first = Count
 };
----------------
I don't entirely understand the motivation for this. I think the idea of LangAS 
is to represent the source ASes while target ASes are reflected in the Map of 
Targets.cpp.


================
Comment at: lib/AST/ASTContext.cpp:9553
+  // alloca.
+  if (AS == LangAS::Default && LangOpts.OpenCL)
+    return getTargetInfo().getDataLayout().getAllocaAddrSpace();
----------------
Here it seems that LangAS::Default is indeed opencl_private?


================
Comment at: test/Sema/invalid-assignment-constant-address-space.c:4
-#define OPENCL_CONSTANT 8388354
-int __attribute__((address_space(OPENCL_CONSTANT))) c[3] = {0};
 
----------------
Is this test even correct? I don't think we can assume that C address spaces 
inherit the same restrictions as OpenCL. Especially that the notion of 
private/local/constant/global is an OpenCL specific thing.

I feel like Clang doesn't behave correctly for C address spaces now.

As for OpenCL I don't see why would anyone use __attribute__((address_space())) 
for constant AS. Especially that it's not part of the spec.


https://reviews.llvm.org/D31404



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to