[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