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

Artem Belevich tra at google.com
Fri Apr 17 17:18:23 PDT 2015


================
Comment at: lib/Sema/SemaDeclAttr.cpp:3474-3475
@@ +3473,4 @@
+
+  assert(!E->isValueDependent() &&
+         "Can't have value-dependent expression at this point.");
+
----------------
rsmith wrote:
> 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.
Considering that we only care whether we can evaluate the expression or not, I've replaced isInstantiationDependent() with isValueDependent() and removed the assert.

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

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:213
@@ +212,3 @@
+
+  Expr *MaxThreads, *MinBlocks = nullptr;
+  ExprResult Result = S.SubstExpr(Attr.getMaxThreads(), TemplateArgs);
----------------
rsmith wrote:
> 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?
OK.

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

http://reviews.llvm.org/D8985

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






More information about the cfe-commits mailing list