[PATCH] [CUDA] Allow using integral non-type template parameters as launch_bounds attribute arguments.

Richard Smith richard at metafoo.co.uk
Fri Apr 17 16:21:55 PDT 2015


LGTM with minor changes.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:3474-3475
@@ +3473,4 @@
+
+  assert(!E->isValueDependent() &&
+         "Can't have value-dependent expression at this point.");
+
----------------
There's no fundamental reason why value dependence must always imply instantiation dependence (and there are cases that should probably be modelled as being value-dependent but not instantiation-dependent, per discussion in the C++ committee, but we don't do so yet). Please also check for value-dependence when you check for instantiation-dependence.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:3507
@@ +3506,3 @@
+
+  // Save dependent expressions in the AST to be instantiated.
+  D->addAttr(::new (Context) CUDALaunchBoundsAttr(
----------------
This comment seems out-of-place since it applies to dependent and non-dependent cases.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:213
@@ +212,3 @@
+
+  Expr *MaxThreads, *MinBlocks = nullptr;
+  ExprResult Result = S.SubstExpr(Attr.getMaxThreads(), TemplateArgs);
----------------
This declaration, while correct, looks suspicious because only the second variable is initialized. Maybe declare `MaxThreads` below at the point where you assign to it, and declare `MinBlocks` immediately before the `if (Attr.getMinBlocks())` line?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:267
@@ -236,2 +266,3 @@
+    }
     // Existing DLL attribute on the instantiation takes precedence.
     if (TmplAttr->getKind() == attr::DLLExport ||
----------------
Add a newline between these, please.

http://reviews.llvm.org/D8985

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list