yaxunl marked 14 inline comments as done.
yaxunl added inline comments.
================
Comment at: lib/CodeGen/CodeGenTypes.cpp:95
-
/// isRecordLayoutComplete - Return true if the specified type is already
----------------
Anastasia wrote:
> Why this change?
Will undo it.
================
Comment at: lib/CodeGen/TargetInfo.cpp:7294
llvm::PointerType *T, QualType QT) const override;
+
};
----------------
rjmccall wrote:
> Spurious newline?
will remove it.
================
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)
----------------
Anastasia wrote:
> 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.
Will move them down.
================
Comment at: test/CodeGenCXX/amdgcn-automatic-variable.cpp:25
+ // CHECK: store i32 2, i32* %[[r1]]
+ int lv1;
+ lv1 = 1;
----------------
Anastasia wrote:
> 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)))`?
As John pointed out, this due to the embedded C specification (ISO/IEC TR
18037). Clang applies it to all languages except OpenCL.
================
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}}
----------------
Anastasia wrote:
> 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.
will add a line for generic.
https://reviews.llvm.org/D32248
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
