Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL271951: [CUDA] Add implicit conversion of __launch_bounds__
arguments to rvalue. (authored by tra).
Changed prior to commit:
http://reviews.llvm.org/D20985?vs=59778&id=59800#toc
Repository:
rL LLVM
http://reviews.llvm.org/D20985
Files:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/CodeGenCUDA/launch-bounds.cu
cfe/trunk/test/SemaCUDA/pr27778.cu
Index: cfe/trunk/test/CodeGenCUDA/launch-bounds.cu
===
--- cfe/trunk/test/CodeGenCUDA/launch-bounds.cu
+++ cfe/trunk/test/CodeGenCUDA/launch-bounds.cu
@@ -79,3 +79,8 @@
}
// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"maxntidx",
// CHECK-NOT: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"minctasm",
+
+const char constchar = 12;
+__global__ void __launch_bounds__(constint, constchar) Kernel8() {}
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"maxntidx", i32 100
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"minctasm", i32 12
Index: cfe/trunk/test/SemaCUDA/pr27778.cu
===
--- cfe/trunk/test/SemaCUDA/pr27778.cu
+++ cfe/trunk/test/SemaCUDA/pr27778.cu
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s
+
+#include "Inputs/cuda.h"
+
+const int constint = 512;
+__launch_bounds__(constint) void TestConstInt(void) {}
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -28,6 +28,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/DelayedDiagnostic.h"
+#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Scope.h"
#include "llvm/ADT/StringExtras.h"
@@ -4039,48 +4040,60 @@
return false;
}
-// Checks whether an argument of launch_bounds attribute is acceptable
-// May output an error.
-static bool checkLaunchBoundsArgument(Sema &S, Expr *E,
- const CUDALaunchBoundsAttr &Attr,
- const unsigned Idx) {
+// Checks whether an argument of launch_bounds attribute is
+// acceptable, performs implicit conversion to Rvalue, and returns
+// non-nullptr Expr result on success. Otherwise, it returns nullptr
+// and may output an error.
+static Expr *makeLaunchBoundsArgExpr(Sema &S, Expr *E,
+ const CUDALaunchBoundsAttr &Attr,
+ const unsigned Idx) {
if (S.DiagnoseUnexpandedParameterPack(E))
-return false;
+return nullptr;
// Accept template arguments for now as they depend on something else.
// We'll get to check them when they eventually get instantiated.
if (E->isValueDependent())
-return true;
+return E;
llvm::APSInt I(64);
if (!E->isIntegerConstantExpr(I, S.Context)) {
S.Diag(E->getExprLoc(), diag::err_attribute_argument_n_type)
<< &Attr << Idx << AANT_ArgumentIntegerConstant << E->getSourceRange();
-return false;
+return nullptr;
}
// Make sure we can fit it in 32 bits.
if (!I.isIntN(32)) {
S.Diag(E->getExprLoc(), diag::err_ice_too_large) << I.toString(10, false)
<< 32 << /* Unsigned */ 1;
-return false;
+return nullptr;
}
if (I < 0)
S.Diag(E->getExprLoc(), diag::warn_attribute_argument_n_negative)
<< &Attr << Idx << E->getSourceRange();
- return true;
+ // We may need to perform implicit conversion of the argument.
+ InitializedEntity Entity = InitializedEntity::InitializeParameter(
+ S.Context, S.Context.getConstType(S.Context.IntTy), /*consume*/ false);
+ ExprResult ValArg = S.PerformCopyInitialization(Entity, SourceLocation(), E);
+ assert(!ValArg.isInvalid() &&
+ "Unexpected PerformCopyInitialization() failure.");
+
+ return ValArg.getAs();
}
void Sema::AddLaunchBoundsAttr(SourceRange AttrRange, Decl *D, Expr *MaxThreads,
Expr *MinBlocks, unsigned SpellingListIndex) {
CUDALaunchBoundsAttr TmpAttr(AttrRange, Context, MaxThreads, MinBlocks,
SpellingListIndex);
-
- if (!checkLaunchBoundsArgument(*this, MaxThreads, TmpAttr, 0))
+ MaxThreads = makeLaunchBoundsArgExpr(*this, MaxThreads, TmpAttr, 0);
+ if (MaxThreads == nullptr)
return;
- if (MinBlocks && !checkLaunchBoundsArgument(*this, MinBlocks, TmpAttr, 1))
-return;
+ if (MinBlocks) {
+MinBlocks = makeLaunchBoundsArgExpr(*this, MinBlocks, TmpAttr, 1);
+if (MinBlocks == nullptr)
+ return;
+ }
D->addAttr(::new (Context) CUDALaunchBoundsAttr(
AttrRange, Context, MaxThreads, MinBlocks, SpellingListIndex));
___
cfe-commits mailing list
cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
tra marked an inline comment as done. tra added a comment. http://reviews.llvm.org/D20985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
tra updated this revision to Diff 59778.
tra added a comment.
Replaced if() with assert() to catch unexpected PerformCopyInitialization()
failures.
http://reviews.llvm.org/D20985
Files:
lib/Sema/SemaDeclAttr.cpp
test/CodeGenCUDA/launch-bounds.cu
test/SemaCUDA/pr27778.cu
Index: test/SemaCUDA/pr27778.cu
===
--- /dev/null
+++ test/SemaCUDA/pr27778.cu
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s
+
+#include "Inputs/cuda.h"
+
+const int constint = 512;
+__launch_bounds__(constint) void TestConstInt(void) {}
Index: test/CodeGenCUDA/launch-bounds.cu
===
--- test/CodeGenCUDA/launch-bounds.cu
+++ test/CodeGenCUDA/launch-bounds.cu
@@ -79,3 +79,8 @@
}
// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"maxntidx",
// CHECK-NOT: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"minctasm",
+
+const char constchar = 12;
+__global__ void __launch_bounds__(constint, constchar) Kernel8() {}
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"maxntidx", i32 100
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"minctasm", i32 12
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -28,6 +28,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/DelayedDiagnostic.h"
+#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Scope.h"
#include "llvm/ADT/StringExtras.h"
@@ -4039,48 +4040,60 @@
return false;
}
-// Checks whether an argument of launch_bounds attribute is acceptable
-// May output an error.
-static bool checkLaunchBoundsArgument(Sema &S, Expr *E,
- const CUDALaunchBoundsAttr &Attr,
- const unsigned Idx) {
+// Checks whether an argument of launch_bounds attribute is
+// acceptable, performs implicit conversion to Rvalue, and returns
+// non-nullptr Expr result on success. Otherwise, it returns nullptr
+// and may output an error.
+static Expr *makeLaunchBoundsArgExpr(Sema &S, Expr *E,
+ const CUDALaunchBoundsAttr &Attr,
+ const unsigned Idx) {
if (S.DiagnoseUnexpandedParameterPack(E))
-return false;
+return nullptr;
// Accept template arguments for now as they depend on something else.
// We'll get to check them when they eventually get instantiated.
if (E->isValueDependent())
-return true;
+return E;
llvm::APSInt I(64);
if (!E->isIntegerConstantExpr(I, S.Context)) {
S.Diag(E->getExprLoc(), diag::err_attribute_argument_n_type)
<< &Attr << Idx << AANT_ArgumentIntegerConstant << E->getSourceRange();
-return false;
+return nullptr;
}
// Make sure we can fit it in 32 bits.
if (!I.isIntN(32)) {
S.Diag(E->getExprLoc(), diag::err_ice_too_large) << I.toString(10, false)
<< 32 << /* Unsigned */ 1;
-return false;
+return nullptr;
}
if (I < 0)
S.Diag(E->getExprLoc(), diag::warn_attribute_argument_n_negative)
<< &Attr << Idx << E->getSourceRange();
- return true;
+ // We may need to perform implicit conversion of the argument.
+ InitializedEntity Entity = InitializedEntity::InitializeParameter(
+ S.Context, S.Context.getConstType(S.Context.IntTy), /*consume*/ false);
+ ExprResult ValArg = S.PerformCopyInitialization(Entity, SourceLocation(), E);
+ assert(!ValArg.isInvalid() &&
+ "Unexpected PerformCopyInitialization() failure.");
+
+ return ValArg.getAs();
}
void Sema::AddLaunchBoundsAttr(SourceRange AttrRange, Decl *D, Expr *MaxThreads,
Expr *MinBlocks, unsigned SpellingListIndex) {
CUDALaunchBoundsAttr TmpAttr(AttrRange, Context, MaxThreads, MinBlocks,
SpellingListIndex);
-
- if (!checkLaunchBoundsArgument(*this, MaxThreads, TmpAttr, 0))
+ MaxThreads = makeLaunchBoundsArgExpr(*this, MaxThreads, TmpAttr, 0);
+ if (MaxThreads == nullptr)
return;
- if (MinBlocks && !checkLaunchBoundsArgument(*this, MinBlocks, TmpAttr, 1))
-return;
+ if (MinBlocks) {
+MinBlocks = makeLaunchBoundsArgExpr(*this, MinBlocks, TmpAttr, 1);
+if (MinBlocks == nullptr)
+ return;
+ }
D->addAttr(::new (Context) CUDALaunchBoundsAttr(
AttrRange, Context, MaxThreads, MinBlocks, SpellingListIndex));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
jlebar added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:4079 @@ +4078,3 @@ + if (ValArg.isInvalid()) +return nullptr; + OK, so then we want an assert, not an if? http://reviews.llvm.org/D20985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
tra marked 3 inline comments as done. Comment at: lib/Sema/SemaDeclAttr.cpp:4046 @@ +4045,3 @@ +// non-nullptr Expr result on success. Returns nullptr otherwise and +// may output an error. +static Expr *makeLaunchBoundsArgExpr(Sema &S, Expr *E, jlebar wrote: > Presumably it "returns nullptr and outputs an error" otherwise? Like, we get > nullptr iff it outputs an error? It returns nullptr without error message in case of unexpanded parameter pack. Arguments for that case will be checked after template instantiation is done. See instantiateDependentCUDALaunchBoundsAttr() in SemaTemplateInstantiateDecl.cpp. Comment at: lib/Sema/SemaDeclAttr.cpp:4079 @@ +4078,3 @@ + if (ValArg.isInvalid()) +return nullptr; + jlebar wrote: > Do we need to output an error here, or is does PerformCopyInitialization do > so for us? In any case, is it covered by a test? Actually, by this point we've verified that Expr is an ICE. I assume that PerformCopyInitialization() should always succeed for such an expression. http://reviews.llvm.org/D20985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
tra updated this revision to Diff 59631.
tra marked an inline comment as done.
tra added a comment.
Rephrased comments
http://reviews.llvm.org/D20985
Files:
lib/Sema/SemaDeclAttr.cpp
test/CodeGenCUDA/launch-bounds.cu
test/SemaCUDA/pr27778.cu
Index: test/SemaCUDA/pr27778.cu
===
--- /dev/null
+++ test/SemaCUDA/pr27778.cu
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s
+
+#include "Inputs/cuda.h"
+
+const int constint = 512;
+__launch_bounds__(constint) void TestConstInt(void) {}
Index: test/CodeGenCUDA/launch-bounds.cu
===
--- test/CodeGenCUDA/launch-bounds.cu
+++ test/CodeGenCUDA/launch-bounds.cu
@@ -79,3 +79,8 @@
}
// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"maxntidx",
// CHECK-NOT: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"minctasm",
+
+const char constchar = 12;
+__global__ void __launch_bounds__(constint, constchar) Kernel8() {}
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"maxntidx", i32 100
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"minctasm", i32 12
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -28,6 +28,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/DelayedDiagnostic.h"
+#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Scope.h"
#include "llvm/ADT/StringExtras.h"
@@ -4039,48 +4040,60 @@
return false;
}
-// Checks whether an argument of launch_bounds attribute is acceptable
-// May output an error.
-static bool checkLaunchBoundsArgument(Sema &S, Expr *E,
- const CUDALaunchBoundsAttr &Attr,
- const unsigned Idx) {
+// Checks whether an argument of launch_bounds attribute is
+// acceptable, performs implicit conversion to Rvalue, and returns
+// non-nullptr Expr result on success. Otherwise, it returns nullptr
+// and may output an error.
+static Expr *makeLaunchBoundsArgExpr(Sema &S, Expr *E,
+ const CUDALaunchBoundsAttr &Attr,
+ const unsigned Idx) {
if (S.DiagnoseUnexpandedParameterPack(E))
-return false;
+return nullptr;
// Accept template arguments for now as they depend on something else.
// We'll get to check them when they eventually get instantiated.
if (E->isValueDependent())
-return true;
+return E;
llvm::APSInt I(64);
if (!E->isIntegerConstantExpr(I, S.Context)) {
S.Diag(E->getExprLoc(), diag::err_attribute_argument_n_type)
<< &Attr << Idx << AANT_ArgumentIntegerConstant << E->getSourceRange();
-return false;
+return nullptr;
}
// Make sure we can fit it in 32 bits.
if (!I.isIntN(32)) {
S.Diag(E->getExprLoc(), diag::err_ice_too_large) << I.toString(10, false)
<< 32 << /* Unsigned */ 1;
-return false;
+return nullptr;
}
if (I < 0)
S.Diag(E->getExprLoc(), diag::warn_attribute_argument_n_negative)
<< &Attr << Idx << E->getSourceRange();
- return true;
+ // We may need to perform implicit conversion of the argument.
+ InitializedEntity Entity = InitializedEntity::InitializeParameter(
+ S.Context, S.Context.getConstType(S.Context.IntTy), /*consume*/ false);
+ ExprResult ValArg = S.PerformCopyInitialization(Entity, SourceLocation(), E);
+ if (ValArg.isInvalid())
+return nullptr;
+
+ return ValArg.getAs();
}
void Sema::AddLaunchBoundsAttr(SourceRange AttrRange, Decl *D, Expr *MaxThreads,
Expr *MinBlocks, unsigned SpellingListIndex) {
CUDALaunchBoundsAttr TmpAttr(AttrRange, Context, MaxThreads, MinBlocks,
SpellingListIndex);
-
- if (!checkLaunchBoundsArgument(*this, MaxThreads, TmpAttr, 0))
+ MaxThreads = makeLaunchBoundsArgExpr(*this, MaxThreads, TmpAttr, 0);
+ if (MaxThreads == nullptr)
return;
- if (MinBlocks && !checkLaunchBoundsArgument(*this, MinBlocks, TmpAttr, 1))
-return;
+ if (MinBlocks) {
+MinBlocks = makeLaunchBoundsArgExpr(*this, MinBlocks, TmpAttr, 1);
+if (MinBlocks == nullptr)
+ return;
+ }
D->addAttr(::new (Context) CUDALaunchBoundsAttr(
AttrRange, Context, MaxThreads, MinBlocks, SpellingListIndex));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
jlebar accepted this revision. This revision is now accepted and ready to land. Comment at: lib/Sema/SemaDeclAttr.cpp:4044 @@ +4043,3 @@ +// Checks whether an argument of launch_bounds attribute is +// acceptable, performs implicit conversion to Rvalue and returns +// non-nullptr Expr result on success. Returns nullptr otherwise and Nit, Oxford comma helps some here. Comment at: lib/Sema/SemaDeclAttr.cpp:4046 @@ +4045,3 @@ +// non-nullptr Expr result on success. Returns nullptr otherwise and +// may output an error. +static Expr *makeLaunchBoundsArgExpr(Sema &S, Expr *E, Presumably it "returns nullptr and outputs an error" otherwise? Like, we get nullptr iff it outputs an error? Comment at: lib/Sema/SemaDeclAttr.cpp:4079 @@ +4078,3 @@ + if (ValArg.isInvalid()) +return nullptr; + Do we need to output an error here, or is does PerformCopyInitialization do so for us? In any case, is it covered by a test? http://reviews.llvm.org/D20985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
tra updated this revision to Diff 59624.
tra added a comment.
Addressed Justin's comments.
http://reviews.llvm.org/D20985
Files:
lib/Sema/SemaDeclAttr.cpp
test/CodeGenCUDA/launch-bounds.cu
test/SemaCUDA/pr27778.cu
Index: test/SemaCUDA/pr27778.cu
===
--- /dev/null
+++ test/SemaCUDA/pr27778.cu
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s
+
+#include "Inputs/cuda.h"
+
+const int constint = 512;
+__launch_bounds__(constint) void TestConstInt(void) {}
Index: test/CodeGenCUDA/launch-bounds.cu
===
--- test/CodeGenCUDA/launch-bounds.cu
+++ test/CodeGenCUDA/launch-bounds.cu
@@ -79,3 +79,8 @@
}
// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"maxntidx",
// CHECK-NOT: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"minctasm",
+
+const char constchar = 12;
+__global__ void __launch_bounds__(constint, constchar) Kernel8() {}
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"maxntidx", i32 100
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"minctasm", i32 12
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -28,6 +28,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/DelayedDiagnostic.h"
+#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Scope.h"
#include "llvm/ADT/StringExtras.h"
@@ -4039,48 +4040,60 @@
return false;
}
-// Checks whether an argument of launch_bounds attribute is acceptable
-// May output an error.
-static bool checkLaunchBoundsArgument(Sema &S, Expr *E,
- const CUDALaunchBoundsAttr &Attr,
- const unsigned Idx) {
+// Checks whether an argument of launch_bounds attribute is
+// acceptable, performs implicit conversion to Rvalue and returns
+// non-nullptr Expr result on success. Returns nullptr otherwise and
+// may output an error.
+static Expr *makeLaunchBoundsArgExpr(Sema &S, Expr *E,
+ const CUDALaunchBoundsAttr &Attr,
+ const unsigned Idx) {
if (S.DiagnoseUnexpandedParameterPack(E))
-return false;
+return nullptr;
// Accept template arguments for now as they depend on something else.
// We'll get to check them when they eventually get instantiated.
if (E->isValueDependent())
-return true;
+return E;
llvm::APSInt I(64);
if (!E->isIntegerConstantExpr(I, S.Context)) {
S.Diag(E->getExprLoc(), diag::err_attribute_argument_n_type)
<< &Attr << Idx << AANT_ArgumentIntegerConstant << E->getSourceRange();
-return false;
+return nullptr;
}
// Make sure we can fit it in 32 bits.
if (!I.isIntN(32)) {
S.Diag(E->getExprLoc(), diag::err_ice_too_large) << I.toString(10, false)
<< 32 << /* Unsigned */ 1;
-return false;
+return nullptr;
}
if (I < 0)
S.Diag(E->getExprLoc(), diag::warn_attribute_argument_n_negative)
<< &Attr << Idx << E->getSourceRange();
- return true;
+ // We may need to perform implicit conversion of the argument.
+ InitializedEntity Entity = InitializedEntity::InitializeParameter(
+ S.Context, S.Context.getConstType(S.Context.IntTy), /*consume*/ false);
+ ExprResult ValArg = S.PerformCopyInitialization(Entity, SourceLocation(), E);
+ if (ValArg.isInvalid())
+return nullptr;
+
+ return ValArg.getAs();
}
void Sema::AddLaunchBoundsAttr(SourceRange AttrRange, Decl *D, Expr *MaxThreads,
Expr *MinBlocks, unsigned SpellingListIndex) {
CUDALaunchBoundsAttr TmpAttr(AttrRange, Context, MaxThreads, MinBlocks,
SpellingListIndex);
-
- if (!checkLaunchBoundsArgument(*this, MaxThreads, TmpAttr, 0))
+ MaxThreads = makeLaunchBoundsArgExpr(*this, MaxThreads, TmpAttr, 0);
+ if (MaxThreads == nullptr)
return;
- if (MinBlocks && !checkLaunchBoundsArgument(*this, MinBlocks, TmpAttr, 1))
-return;
+ if (MinBlocks) {
+MinBlocks = makeLaunchBoundsArgExpr(*this, MinBlocks, TmpAttr, 1);
+if (MinBlocks == nullptr)
+ return;
+ }
D->addAttr(::new (Context) CUDALaunchBoundsAttr(
AttrRange, Context, MaxThreads, MinBlocks, SpellingListIndex));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
jlebar added a comment.
In http://reviews.llvm.org/D20985#448836, @tra wrote:
> In http://reviews.llvm.org/D20985#448822, @jlebar wrote:
>
> > How is this different from test/SemaCUDA/launch_bounds.cu:27-28? It does
> >
> > const int constint = 512;
> > __launch_bounds__(constint) void TestConstInt(void);
> >
> >
> > which looks verbatim the same as this testcase.
>
>
> Existing test is a declaration of the function which did not trigger the
> crash.
> Second issue is that -verify interferes with reproduction case -- the crash
> does not happen if any //expect-* are seen before it.
> Plus, the outcome of the failing test is a crash which would prevent reports
> of other failures.
> Separate test file makes the crash isolated and reliably reproducible.
Got it, thanks.
Should we have a test that passes a char or a short and ensures that we do the
correct implicit conversion there?
Comment at: lib/Sema/SemaDeclAttr.cpp:4045
@@ -4043,4 +4044,3 @@
// May output an error.
-static bool checkLaunchBoundsArgument(Sema &S, Expr *E,
- const CUDALaunchBoundsAttr &Attr,
- const unsigned Idx) {
+static Expr *checkLaunchBoundsArgument(Sema &S, Expr *E,
+ const CUDALaunchBoundsAttr &Attr,
Should we update this name and comment?
http://reviews.llvm.org/D20985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
tra added a comment. In http://reviews.llvm.org/D20985#448822, @jlebar wrote: > How is this different from test/SemaCUDA/launch_bounds.cu:27-28? It does > > const int constint = 512; > __launch_bounds__(constint) void TestConstInt(void); > > > which looks verbatim the same as this testcase. Existing test is a declaration of the function which did not trigger the crash. Second issue is that -verify interferes with reproduction case -- the crash does not happen if any //expect-* are seen before it. Plus, the outcome of the failing test is a crash which would prevent reports of other failures. Separate test file makes the crash isolated and reliably reproducible. http://reviews.llvm.org/D20985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
jlebar added a comment. How is this different from test/SemaCUDA/launch_bounds.cu:27-28? It does const int constint = 512; __launch_bounds__(constint) void TestConstInt(void); which looks verbatim the same as this testcase. http://reviews.llvm.org/D20985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20985: [CUDA] Add implicit conversion of __launch_bounds__ arguments to rvalue.
tra created this revision.
tra added reviewers: rsmith, jlebar.
tra added a subscriber: cfe-commits.
Fixes clang crash reported in PR27778.
http://reviews.llvm.org/D20985
Files:
lib/Sema/SemaDeclAttr.cpp
test/CodeGenCUDA/launch-bounds.cu
test/SemaCUDA/pr27778.cu
Index: test/SemaCUDA/pr27778.cu
===
--- /dev/null
+++ test/SemaCUDA/pr27778.cu
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s
+
+#include "Inputs/cuda.h"
+
+const int constint = 512;
+__launch_bounds__(constint) void TestConstInt(void) {}
Index: test/CodeGenCUDA/launch-bounds.cu
===
--- test/CodeGenCUDA/launch-bounds.cu
+++ test/CodeGenCUDA/launch-bounds.cu
@@ -79,3 +79,10 @@
}
// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"maxntidx",
// CHECK-NOT: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel7{{.*}}, !"minctasm",
+
+const int max_threads_per_block = 11;
+const int min_blocks_per_mp = 12;
+__global__ void __launch_bounds__(max_threads_per_block, min_blocks_per_mp)
+Kernel8() {}
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"maxntidx", i32 11
+// CHECK: !{{[0-9]+}} = !{void ()* @{{.*}}Kernel8{{.*}}, !"minctasm", i32 12
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -28,6 +28,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/DelayedDiagnostic.h"
+#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Scope.h"
#include "llvm/ADT/StringExtras.h"
@@ -4041,46 +4042,56 @@
// Checks whether an argument of launch_bounds attribute is acceptable
// May output an error.
-static bool checkLaunchBoundsArgument(Sema &S, Expr *E,
- const CUDALaunchBoundsAttr &Attr,
- const unsigned Idx) {
+static Expr *checkLaunchBoundsArgument(Sema &S, Expr *E,
+ const CUDALaunchBoundsAttr &Attr,
+ const unsigned Idx) {
if (S.DiagnoseUnexpandedParameterPack(E))
-return false;
+return nullptr;
// Accept template arguments for now as they depend on something else.
// We'll get to check them when they eventually get instantiated.
if (E->isValueDependent())
-return true;
+return E;
llvm::APSInt I(64);
if (!E->isIntegerConstantExpr(I, S.Context)) {
S.Diag(E->getExprLoc(), diag::err_attribute_argument_n_type)
<< &Attr << Idx << AANT_ArgumentIntegerConstant << E->getSourceRange();
-return false;
+return nullptr;
}
// Make sure we can fit it in 32 bits.
if (!I.isIntN(32)) {
S.Diag(E->getExprLoc(), diag::err_ice_too_large) << I.toString(10, false)
<< 32 << /* Unsigned */ 1;
-return false;
+return nullptr;
}
if (I < 0)
S.Diag(E->getExprLoc(), diag::warn_attribute_argument_n_negative)
<< &Attr << Idx << E->getSourceRange();
- return true;
+ // We may need to perform implicit conversion of the argument.
+ InitializedEntity Entity = InitializedEntity::InitializeParameter(
+ S.Context, S.Context.getConstType(S.Context.IntTy), /*consume*/ false);
+ ExprResult ValArg = S.PerformCopyInitialization(Entity, SourceLocation(), E);
+ if (ValArg.isInvalid())
+return nullptr;
+
+ return ValArg.getAs();
}
void Sema::AddLaunchBoundsAttr(SourceRange AttrRange, Decl *D, Expr *MaxThreads,
Expr *MinBlocks, unsigned SpellingListIndex) {
CUDALaunchBoundsAttr TmpAttr(AttrRange, Context, MaxThreads, MinBlocks,
SpellingListIndex);
-
- if (!checkLaunchBoundsArgument(*this, MaxThreads, TmpAttr, 0))
+ MaxThreads = checkLaunchBoundsArgument(*this, MaxThreads, TmpAttr, 0);
+ if (MaxThreads == nullptr)
return;
- if (MinBlocks && !checkLaunchBoundsArgument(*this, MinBlocks, TmpAttr, 1))
-return;
+ if (MinBlocks) {
+MinBlocks = checkLaunchBoundsArgument(*this, MinBlocks, TmpAttr, 1);
+if (MinBlocks == nullptr)
+ return;
+ }
D->addAttr(::new (Context) CUDALaunchBoundsAttr(
AttrRange, Context, MaxThreads, MinBlocks, SpellingListIndex));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
