rjmccall added a comment.

Looks like you haven't introduced proper constants in the header for scopes.



================
Comment at: docs/LanguageExtensions.rst:1855
+atomic builtins are in ``explicit`` form of the corresponding OpenCL 2.0
+builtin function, and are named with a ``__opencl__`` prefix.)
 
----------------
1. "an", not "in".
2. There's no need to put "explicit" in `` quotes.
3. The prefix you're using is "__opencl_", with only one trailing underscore.


================
Comment at: lib/CodeGen/CGAtomic.cpp:736
     OrderFail = EmitScalarExpr(E->getOrderFail());
-    if (E->getNumSubExprs() == 6)
+    if (E->getNumSubExprs() == 7)
       IsWeak = EmitScalarExpr(E->getWeak());
----------------
This is equivalent to checking for the two AO kinds, right?  Probably better to 
just do that.


================
Comment at: lib/CodeGen/CGExpr.cpp:50
+llvm::Value *CodeGenFunction::EmitCastToVoidPtr(llvm::Value *value,
+                                                bool CastToGenericAddrSpace) {
+  unsigned addressSpace;
----------------
I think it would be better to keep this function simple and just add the cast 
outside in the two places you need it.

Also, please remember to use the target hook instead of using an addrspacecast 
directly.


================
Comment at: lib/Sema/SemaChecking.cpp:3046
       // The order(s) are always converted to int.
       Ty = Context.IntTy;
     }
----------------
This clause now covers the scope argument as well.


================
Comment at: lib/Sema/SemaChecking.cpp:3060
+  if (IsOpenCL) {
+    Scope = TheCall->getArg(TheCall->getNumArgs() - 1);
+  } else {
----------------
You need to check that is a constant expression in the correct range, and you 
should add a test for that.


================
Comment at: lib/Sema/SemaChecking.cpp:3062
+  } else {
+    Scope = IntegerLiteral::Create(
+        Context, llvm::APInt(Context.getTypeSize(Context.IntTy), (uint64_t)1),
----------------
Please document the meaning of 1.


https://reviews.llvm.org/D28691



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

Reply via email to