Anastasia added inline comments.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8127
"invalid prototype, variadic arguments are not allowed in OpenCL">;
+def err_requires_extension : Error<
+ "use of %select{declaration|type }0%1 requires %2 extension to be enabled">;
----------------
Could we rename err_requires_extension -> err_opencl_requires_extension
================
Comment at: lib/Parse/ParsePragma.cpp:461
+ };
+ typedef std::pair<const IdentifierInfo *, OpenCLExtState> OpenCLExtData;
}
----------------
yaxunl wrote:
> Anastasia wrote:
> > yaxunl wrote:
> > > Anastasia wrote:
> > > > Could we use PointerIntPair still but with 2 bits?
> > > Using PointerIntPair with 2 bits requires IdentifierInfo to be aligned at
> > > 4 bytes, however this is not true. IdentifierInfo has no explicit
> > > alignment specifier, therefore it can only hold 1 bit in PointerIntPair.
> > Based on its structure with having a pointer member I think it's reasonable
> > to assume at least 4 bytes alignment... Although this doesn't seem to
> > affect the performance anyways.
> You are right. The alignment of IdentifierInfo should be 8 on most systems.
>
> However in IdentifierTable.h there is
>
>
> ```
> // Provide PointerLikeTypeTraits for IdentifierInfo pointers, which
> // are not guaranteed to be 8-byte aligned.
> template<>
> class PointerLikeTypeTraits<clang::IdentifierInfo*> {
> public:
> static inline void *getAsVoidPointer(clang::IdentifierInfo* P) {
> return P;
> }
> static inline clang::IdentifierInfo *getFromVoidPointer(void *P) {
> return static_cast<clang::IdentifierInfo*>(P);
> }
> enum { NumLowBitsAvailable = 1 };
> };
>
> ```
> Due to the above code there is only 1 bit available for when using
> PointerIntPair with IdentifierInfo*. My guess is that alignment of
> IdentifierInfo is not 8 on certain systems so some one intentionally defined
> the above code to prevent using PointerIntPair with IdentifierInfo* for more
> than 1 bit.
>
Not too critical, but it's certainly not 8 byte aligned for 32 bit
architectures. It should although be at least 4 bytes aligned in my opinion.
This code seems to be old, and the comment is not very precise to be honest.
================
Comment at: lib/Sema/Sema.cpp:1614
+ Decl = TagT->getDecl();
+ auto DL = OpenCLDeclExtMap.find(Decl);
+ if (DL != OpenCLDeclExtMap.end()) {
----------------
this bit (up to line 1623) seems to be repeated in multiple places... wondering
if we could use a helper function instead perhaps with the templates?
================
Comment at: lib/Serialization/ASTReader.cpp:3170
+ OpenCLExtensions.OptMap[Name] = {
+ static_cast<bool>(Record[I++]),
+ static_cast<bool>(Record[I++]),
----------------
Can you put a comment explaining which element each line corresponds to?
https://reviews.llvm.org/D21698
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
