[PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Anastasia Stulova via Phabricator via cfe-commits]

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