Anastasia added inline comments.

================
Comment at: lib/CodeGen/CodeGenTypes.cpp:95
 
-
 /// isRecordLayoutComplete - Return true if the specified type is already
----------------
Why this change?


================
Comment at: lib/CodeGen/TargetInfo.cpp:411
+    CodeGen::CodeGenFunction &CGF, llvm::Value *Src, unsigned SrcAddr,
+    unsigned DestAddr, llvm::Type *DestTy, bool isNonNull) const {
   // Since target may map different address spaces in AST to the same address
----------------
Seems like some parameters are unused.


================
Comment at: lib/Sema/SemaDecl.cpp:7206
+    assert(T.getAddressSpace() != LangAS::opencl_constant);
+    if (T.getAddressSpace() == LangAS::opencl_global) {
+      Diag(NewVD->getLocation(), diag::err_opencl_function_variable)
----------------
I think it was nicer that all OpenCL related changes were grouped in one spot 
under the same if statement. Even though it isn't quite done everywhere. But 
this change makes them even more scattered.


================
Comment at: test/CodeGenCXX/amdgcn-automatic-variable.cpp:25
+  // CHECK: store i32 2, i32* %[[r1]]
+  int lv1;
+  lv1 = 1;
----------------
yaxunl wrote:
> yaxunl wrote:
> > Anastasia wrote:
> > > I am wondering if all these different test cases are really needed. Are 
> > > we testing any different program paths from the patch?
> > > 
> > > Also would it make sense to add a test case with an object in an AS 
> > > different to 0 (i.e. with `__attribute__((address_space(n)))`)
> > I think at least I should cover the typical use cases of auto var.
> > 
> > I will add a test for __attribute__((address_space(n))))
> Sorry. I just checked that C++ does not allow 
> __attribute__((address_space(n))) on automatic var.
I thought address space was undocumented extension. So essentially you can't 
have a cast to any address space but a default here? Would it work for the C 
instead to add `attribute((address_space(n)))`?


================
Comment at: test/SemaOpenCL/storageclass-cl20.cl:16
+  global int L3;                              // expected-error{{function 
scope variable cannot be declared in global address space}}
+  __attribute__((address_space(100))) int L4; // expected-error{{automatic 
variable qualified with an invalid address space}}
 
----------------
I am guessing generic should be rejected here as well... although it should 
only apply to pointers. But it seems we don't handle this properly.


https://reviews.llvm.org/D32248



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

Reply via email to