rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaType.cpp:2188
+ !Context.getTargetInfo().isVLASupported() &&
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
tra wrote:
> rjmccall w
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319201: [CUDA] Report "unsupported VLA" errors only on
device side. (authored by tra).
Changed prior to commit:
https://reviews.llvm.org/D40275?vs=123831&id=124607#toc
Repository:
rL LLVM
https://re
tra added inline comments.
Comment at: clang/lib/Sema/SemaType.cpp:2188
+ !Context.getTargetInfo().isVLASupported() &&
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
rjmccall wrote:
> tra wrote:
Hahnfeld accepted this revision.
Hahnfeld added a comment.
This revision is now accepted and ready to land.
I'm fine if @rjmccall is fine - but looks like I need to accept this revision
because I requested changes in the past?
https://reviews.llvm.org/D40275
_
rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaType.cpp:2188
+ !Context.getTargetInfo().isVLASupported() &&
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
tra wrote:
> tra wrote:
rjmccall accepted this revision.
rjmccall added a comment.
In https://reviews.llvm.org/D40275#937010, @tra wrote:
> In https://reviews.llvm.org/D40275#933253, @tra wrote:
>
> > @rjmccall : are you OK with this approach? If VLA is not supported by the
> > target, CUDA is handled as a special case
tra added a comment.
In https://reviews.llvm.org/D40275#933253, @tra wrote:
> @rjmccall : are you OK with this approach? If VLA is not supported by the
> target, CUDA is handled as a special case so it can emit deferred diag,
> OpenMP reports an error only if shouldDiagnoseTargetSupportFromOpen
tra added a comment.
@rjmccall : are you OK with this approach? If VLA is not supported by the
target, CUDA is handled as a special case so it can emit deferred diag, OpenMP
reports an error only if shouldDiagnoseTargetSupportFromOpenMP() allows it, and
everything else does so unconditionally.
tra updated this revision to Diff 123831.
tra added a comment.
Updated CUDA tests
https://reviews.llvm.org/D40275
Files:
clang/lib/Sema/SemaType.cpp
clang/test/SemaCUDA/call-stack-for-deferred-err.cu
clang/test/SemaCUDA/no-call-stack-for-immediate-errs.cu
clang/test/SemaCUDA/vla.cu
In
tra added a comment.
In https://reviews.llvm.org/D40275#931947, @hfinkel wrote:
> > When Sema sees this code during compilation, it can not tell whether there
> > is an error. Calling foo from the host code is perfectly valid. Calling it
> > from device code is not. CUDADiagIfDeviceCode creates
tra added inline comments.
Comment at: clang/lib/Sema/SemaType.cpp:2188
+ !Context.getTargetInfo().isVLASupported() &&
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
tra wrote:
> rjmccall wrote:
tra updated this revision to Diff 123823.
tra added a comment.
Updated to partially address rjmccall@ comments.
https://reviews.llvm.org/D40275
Files:
clang/lib/Sema/SemaType.cpp
clang/test/SemaCUDA/vla.cu
Index: clang/test/SemaCUDA/vla.cu
=
hfinkel added a comment.
> When Sema sees this code during compilation, it can not tell whether there is
> an error. Calling foo from the host code is perfectly valid. Calling it from
> device code is not. CUDADiagIfDeviceCode creates 'postponed' diagnostics
> which only gets emitted if we ever
tra added inline comments.
Comment at: clang/lib/Sema/SemaType.cpp:2188
+ !Context.getTargetInfo().isVLASupported() &&
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
rjmccall wrote:
> Please wri
rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaType.cpp:2188
+ !Context.getTargetInfo().isVLASupported() &&
+ shouldDiagnoseTargetSupportFromOpenMP()) {
+ // Some targets don't support VLAs.
Please write this check
tra updated this revision to Diff 123694.
tra added a comment.
Updates CUDA's VLA test to use nvptx triple.
https://reviews.llvm.org/D40275
Files:
clang/lib/Sema/SemaType.cpp
clang/test/SemaCUDA/vla.cu
Index: clang/test/SemaCUDA/vla.cu
=
Hahnfeld added a comment.
In https://reviews.llvm.org/D40275#930981, @tra wrote:
> In https://reviews.llvm.org/D40275#930948, @Hahnfeld wrote:
>
> > In https://reviews.llvm.org/D39505 @rjmccall requested that the check
> > should be made independent of the language. To preserve this, I think the
tra added a comment.
In https://reviews.llvm.org/D40275#930982, @Hahnfeld wrote:
> And please add a regression test which is apparently missing for the case
> that a VLA is **NOT** diagnosed in CUDA mode
Hmm. We do have test/SemaCUDA/vla.cu which should've triggered the error. Let
me see why
tra added a comment.
In https://reviews.llvm.org/D40275#930948, @Hahnfeld wrote:
> In https://reviews.llvm.org/D39505 @rjmccall requested that the check should
> be made independent of the language. To preserve this, I think the CUDA
> specific checks should be added to the generic case instead
Hahnfeld added a comment.
And please add a regression test which is apparently missing for the case that
a VLA is **NOT** diagnosed in CUDA mode
https://reviews.llvm.org/D40275
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.ll
Hahnfeld requested changes to this revision.
Hahnfeld added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D39505 @rjmccall requested that the check should be
made independent of the language. To preserve this, I think the CUDA specific
checks should be add
tra updated this revision to Diff 123690.
tra added a comment.
Folded OpenCL check under `if (T->isVariableArrayType())`
https://reviews.llvm.org/D40275
Files:
clang/lib/Sema/SemaType.cpp
Index: clang/lib/Sema/SemaType.cpp
===
tra created this revision.
Herald added a subscriber: sanjoy.
This fixes erroneously reported CUDA compilation errors in host-side code
during device-side compilation.
I've also restricted OpenMP-specific checks to trigger only if we're compiling
with OpenMP enabled.
https://reviews.llvm.org/
23 matches
Mail list logo
